From d9a55687b99f4ae7f2a66e9b2fb95e748658aae3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 24 Mar 2026 15:51:21 +0100 Subject: [PATCH] =?utf8?q?refactor:=20audit-driven=20improvements=20?= =?utf8?q?=E2=80=94=20use=20BaseError,=20fix=20barrel=20bypass,=20remove?= =?utf8?q?=20redundant=20console?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - Replace throw new Error with BaseError in OCPP20CertificateManager and OCPP20VariableManager (3 occurrences) - Move getMessageTypeString from OCPPServiceUtils to Helpers, resolving the barrel bypass in ErrorUtils — consumers import via charging-station barrel, tests moved to Helpers.test.ts - Remove redundant console.warn in Helpers (logger.warn already called) - Remove unused chalk import from Helpers --- src/charging-station/ChargingStation.ts | 2 +- src/charging-station/Helpers.ts | 16 +++++++++-- src/charging-station/index.ts | 1 + .../ocpp/2.0/OCPP20CertificateManager.ts | 5 ++-- .../ocpp/2.0/OCPP20VariableManager.ts | 3 ++- .../ocpp/OCPPRequestService.ts | 7 ++--- src/charging-station/ocpp/OCPPServiceUtils.ts | 14 ---------- src/charging-station/ocpp/index.ts | 1 - src/utils/ErrorUtils.ts | 2 +- tests/charging-station/Helpers.test.ts | 20 ++++++++++++++ .../ocpp/OCPPServiceUtils-pure.test.ts | 27 +------------------ 11 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts index fec2fcb6..0ad4fcf3 100644 --- a/src/charging-station/ChargingStation.ts +++ b/src/charging-station/ChargingStation.ts @@ -129,6 +129,7 @@ import { getHashId, getIdTagsFile, getMaxNumberOfEvses, + getMessageTypeString, getNumberOfReservableConnectors, getPhaseRotationValue, hasFeatureProfile, @@ -143,7 +144,6 @@ import { } from './Helpers.js' import { IdTagsCache } from './IdTagsCache.js' import { - getMessageTypeString, OCPP16IncomingRequestService, OCPP16RequestService, OCPP16ResponseService, diff --git a/src/charging-station/Helpers.ts b/src/charging-station/Helpers.ts index 2ae5aedb..e30798d0 100644 --- a/src/charging-station/Helpers.ts +++ b/src/charging-station/Helpers.ts @@ -1,6 +1,5 @@ import type { EventEmitter } from 'node:events' -import chalk from 'chalk' import { addDays, addSeconds, @@ -46,6 +45,7 @@ import { ConnectorStatusEnum, CurrentType, type EvseTemplate, + MessageType, OCPPVersion, RecurrencyKindType, type Reservation, @@ -1026,7 +1026,6 @@ const warnDeprecatedTemplateKey = ( isNotEmptyString(logMsgToAppend) ? `. ${logMsgToAppend}` : '' }` logger.warn(`${logPrefix} ${logMsg}`) - console.warn(`${chalk.green(logPrefix)} ${chalk.yellow(logMsg)}`) } } @@ -1511,3 +1510,16 @@ const getRandomSerialNumberSuffix = (params?: { } return randomSerialNumberSuffix } + +export const getMessageTypeString = (messageType: MessageType | undefined): string => { + switch (messageType) { + case MessageType.CALL_ERROR_MESSAGE: + return 'error' + case MessageType.CALL_MESSAGE: + return 'request' + case MessageType.CALL_RESULT_MESSAGE: + return 'response' + default: + return 'unknown' + } +} diff --git a/src/charging-station/index.ts b/src/charging-station/index.ts index b01ac1b6..7a48f729 100644 --- a/src/charging-station/index.ts +++ b/src/charging-station/index.ts @@ -10,6 +10,7 @@ export { checkChargingStationState, getConnectorChargingProfiles, getIdTagsFile, + getMessageTypeString, hasFeatureProfile, hasPendingReservation, hasPendingReservations, diff --git a/src/charging-station/ocpp/2.0/OCPP20CertificateManager.ts b/src/charging-station/ocpp/2.0/OCPP20CertificateManager.ts index 8de4b883..a81e8857 100644 --- a/src/charging-station/ocpp/2.0/OCPP20CertificateManager.ts +++ b/src/charging-station/ocpp/2.0/OCPP20CertificateManager.ts @@ -6,6 +6,7 @@ import { join, resolve, sep } from 'node:path' import type { ChargingStation } from '../../ChargingStation.js' +import { BaseError } from '../../../exception/index.js' import { type CertificateHashDataChainType, type CertificateHashDataType, @@ -126,7 +127,7 @@ export class OCPP20CertificateManager { issuerCertPem?: string ): CertificateHashDataType { if (!this.validateCertificateFormat(pemData)) { - throw new Error('Invalid PEM certificate format') + throw new BaseError('Invalid PEM certificate format') } const algorithmName = this.getHashAlgorithmName(hashAlgorithm) @@ -626,7 +627,7 @@ export class OCPP20CertificateManager { // Check if resolved path is within the base directory if (!fileResolved.startsWith(baseResolved + sep) && fileResolved !== baseResolved) { - throw new Error( + throw new BaseError( `Path traversal attempt detected: certificate path '${certificateFileName}' resolves outside base directory` ) } diff --git a/src/charging-station/ocpp/2.0/OCPP20VariableManager.ts b/src/charging-station/ocpp/2.0/OCPP20VariableManager.ts index 4a152be5..0b43690b 100644 --- a/src/charging-station/ocpp/2.0/OCPP20VariableManager.ts +++ b/src/charging-station/ocpp/2.0/OCPP20VariableManager.ts @@ -1,5 +1,6 @@ import { millisecondsToSeconds } from 'date-fns' +import { BaseError } from '../../../exception/index.js' import { AttributeEnumType, type ComponentType, @@ -347,7 +348,7 @@ export class OCPP20VariableManager { private getStationId (chargingStation: ChargingStation): string { const stationId = chargingStation.stationInfo?.hashId if (stationId == null) { - throw new Error('ChargingStation has no stationInfo.hashId, cannot identify station') + throw new BaseError('ChargingStation has no stationInfo.hashId, cannot identify station') } return stationId } diff --git a/src/charging-station/ocpp/OCPPRequestService.ts b/src/charging-station/ocpp/OCPPRequestService.ts index 01364af1..3a39a373 100644 --- a/src/charging-station/ocpp/OCPPRequestService.ts +++ b/src/charging-station/ocpp/OCPPRequestService.ts @@ -30,12 +30,9 @@ import { handleSendMessageError, logger, } from '../../utils/index.js' +import { getMessageTypeString } from '../index.js' import { OCPPConstants } from './OCPPConstants.js' -import { - ajvErrorsToErrorType, - convertDateToISOString, - getMessageTypeString, -} from './OCPPServiceUtils.js' +import { ajvErrorsToErrorType, convertDateToISOString } from './OCPPServiceUtils.js' type Ajv = _Ajv.default // eslint-disable-next-line @typescript-eslint/no-redeclare diff --git a/src/charging-station/ocpp/OCPPServiceUtils.ts b/src/charging-station/ocpp/OCPPServiceUtils.ts index c90013a6..2b7b1304 100644 --- a/src/charging-station/ocpp/OCPPServiceUtils.ts +++ b/src/charging-station/ocpp/OCPPServiceUtils.ts @@ -28,7 +28,6 @@ import { type MeasurandPerPhaseSampledValueTemplates, type MeasurandValues, MessageTrigger, - MessageType, type MeterValue, MeterValueContext, MeterValueLocation, @@ -97,19 +96,6 @@ interface SingleValueMeasurandData { value: number } -export const getMessageTypeString = (messageType: MessageType | undefined): string => { - switch (messageType) { - case MessageType.CALL_ERROR_MESSAGE: - return 'error' - case MessageType.CALL_MESSAGE: - return 'request' - case MessageType.CALL_RESULT_MESSAGE: - return 'response' - default: - return 'unknown' - } -} - export const buildStatusNotificationRequest = ( chargingStation: ChargingStation, commandParams: StatusNotificationRequest diff --git a/src/charging-station/ocpp/index.ts b/src/charging-station/ocpp/index.ts index e8d578b0..bfd92858 100644 --- a/src/charging-station/ocpp/index.ts +++ b/src/charging-station/ocpp/index.ts @@ -14,7 +14,6 @@ export { buildMeterValue, buildStatusNotificationRequest, buildTransactionEndMeterValue, - getMessageTypeString, isIdTagAuthorized, sendAndSetConnectorStatus, } from './OCPPServiceUtils.js' diff --git a/src/utils/ErrorUtils.ts b/src/utils/ErrorUtils.ts index 500e4d83..0bea6739 100644 --- a/src/utils/ErrorUtils.ts +++ b/src/utils/ErrorUtils.ts @@ -12,7 +12,7 @@ import type { RequestCommand, } from '../types/index.js' -import { getMessageTypeString } from '../charging-station/ocpp/OCPPServiceUtils.js' +import { getMessageTypeString } from '../charging-station/index.js' import { logger } from './Logger.js' import { isNotEmptyString } from './Utils.js' diff --git a/tests/charging-station/Helpers.test.ts b/tests/charging-station/Helpers.test.ts index 6f522fe4..a064f3bb 100644 --- a/tests/charging-station/Helpers.test.ts +++ b/tests/charging-station/Helpers.test.ts @@ -15,6 +15,7 @@ import { getChargingStationId, getHashId, getMaxNumberOfEvses, + getMessageTypeString, getPhaseRotationValue, hasPendingReservation, hasPendingReservations, @@ -31,6 +32,7 @@ import { type ChargingStationTemplate, type ConnectorStatus, ConnectorStatusEnum, + MessageType, type MeterValue, OCPPVersion, type Reservation, @@ -875,4 +877,22 @@ await describe('Helpers', async () => { assert.strictEqual(connectorStatus.MeterValues.length, 1) }) }) + + await describe('getMessageTypeString', async () => { + await it('should return "request" for MessageType.CALL_MESSAGE', () => { + assert.strictEqual(getMessageTypeString(MessageType.CALL_MESSAGE), 'request') + }) + + await it('should return "response" for MessageType.CALL_RESULT_MESSAGE', () => { + assert.strictEqual(getMessageTypeString(MessageType.CALL_RESULT_MESSAGE), 'response') + }) + + await it('should return "error" for MessageType.CALL_ERROR_MESSAGE', () => { + assert.strictEqual(getMessageTypeString(MessageType.CALL_ERROR_MESSAGE), 'error') + }) + + await it('should return "unknown" for undefined', () => { + assert.strictEqual(getMessageTypeString(undefined), 'unknown') + }) + }) }) diff --git a/tests/charging-station/ocpp/OCPPServiceUtils-pure.test.ts b/tests/charging-station/ocpp/OCPPServiceUtils-pure.test.ts index f74b6c09..dbb50344 100644 --- a/tests/charging-station/ocpp/OCPPServiceUtils-pure.test.ts +++ b/tests/charging-station/ocpp/OCPPServiceUtils-pure.test.ts @@ -3,7 +3,6 @@ * @description Verifies pure utility functions exported from OCPPServiceUtils * * Covers: - * - getMessageTypeString — converts MessageType enum to human-readable string * - ajvErrorsToErrorType — maps AJV validation errors to OCPP ErrorType * - convertDateToISOString — recursively converts Date objects to ISO strings in-place * - OCPPServiceUtils.isConnectorIdValid — validates connector ID ranges @@ -19,15 +18,9 @@ import type { ChargingStation } from '../../../src/charging-station/index.js' import { ajvErrorsToErrorType, convertDateToISOString, - getMessageTypeString, OCPPServiceUtils, } from '../../../src/charging-station/ocpp/OCPPServiceUtils.js' -import { - ErrorType, - IncomingRequestCommand, - type JsonType, - MessageType, -} from '../../../src/types/index.js' +import { ErrorType, IncomingRequestCommand, type JsonType } from '../../../src/types/index.js' import { standardCleanup } from '../../helpers/TestLifecycleHelpers.js' /** @@ -57,24 +50,6 @@ await describe('OCPPServiceUtils — pure functions', async () => { standardCleanup() }) - await describe('getMessageTypeString', async () => { - await it('should return "request" for MessageType.CALL_MESSAGE', () => { - assert.strictEqual(getMessageTypeString(MessageType.CALL_MESSAGE), 'request') - }) - - await it('should return "response" for MessageType.CALL_RESULT_MESSAGE', () => { - assert.strictEqual(getMessageTypeString(MessageType.CALL_RESULT_MESSAGE), 'response') - }) - - await it('should return "error" for MessageType.CALL_ERROR_MESSAGE', () => { - assert.strictEqual(getMessageTypeString(MessageType.CALL_ERROR_MESSAGE), 'error') - }) - - await it('should return "unknown" for undefined', () => { - assert.strictEqual(getMessageTypeString(undefined), 'unknown') - }) - }) - await describe('ajvErrorsToErrorType', async () => { await it('should return FormatViolation for null errors', () => { assert.strictEqual(ajvErrorsToErrorType(null), ErrorType.FORMAT_VIOLATION) -- 2.43.0