fix(charging-station): audit fixes — error handling, OCPP conformance, DRY, type...

- Replace .catch(EMPTY_FUNCTION) with proper error logging (F-001/F-002/F-003)
- Add fail-closed security comment in UIServerSecurity (F-004)
- Convert FIXME to actionable TODO in Bootstrap (F-005)
- Implement relative charging profile duration handling per K01.FR.41-42 (F-006)
- Restart heartbeat and WebSocket ping on template change (F-007)
- Consolidate repeated type casts in convertDeprecatedTemplateKey (F-008)
- Remove duplicate console.warn calls and unused chalk import in UIServerFactory (F-010)
- Extract logPrefix to AbstractUIServer base class, removing duplicates (F-011)
- Extract matchesConfigurationKey predicate in ConfigurationKeyUtils (F-012)
- Add failure counter and error summary for reservation removal (F-013)

diff --git a/src/charging-station/AutomaticTransactionGenerator.ts b/src/charging-station/AutomaticTransactionGenerator.ts
index 8106d29a3c6e33dd951632e0c69940ee54861cfe..22da82a83bbea4c0452d94b6fa2e3f84159011e1 100644 (file)
--- a/src/charging-station/AutomaticTransactionGenerator.ts
+++ b/src/charging-station/AutomaticTransactionGenerator.ts
@@ -101,7 +101,9 @@ export class AutomaticTransactionGenerator {
       throw new BaseError(`Connector ${connectorId.toString()} does not exist`)
     }
     if (this.connectorsStatus.get(connectorId)?.start === false) {
-      this.internalStartConnector(connectorId, stopAbsoluteDuration).catch(Constants.EMPTY_FUNCTION)
+      this.internalStartConnector(connectorId, stopAbsoluteDuration).catch((error: unknown) =>
+        logger.error(`${this.logPrefix(connectorId)} Error while starting connector:`, error)
+      )
     } else if (this.connectorsStatus.get(connectorId)?.start === true) {
       logger.warn(`${this.logPrefix(connectorId)} is already started on connector`)
     }

diff --git a/src/charging-station/Bootstrap.ts b/src/charging-station/Bootstrap.ts
index 9117c09f262cfac9c24cecadd60e2da2454c328b..65831ac5c04daddc9cc8a0435a33e352ada88b84 100644 (file)
--- a/src/charging-station/Bootstrap.ts
+++ b/src/charging-station/Bootstrap.ts
@@ -553,7 +553,7 @@ export class Bootstrap extends EventEmitter {
       this.uiServerStarted = false
     }
     this.initializeCounters()
-    // FIXME: initialize worker implementation only if the worker section has changed
+    // TODO: compare worker configuration hash to skip unnecessary re-initialization
     this.initializeWorkerImplementation(
       Configuration.getConfigurationSection<WorkerConfiguration>(ConfigurationSection.worker)
     )
@@ -616,7 +616,9 @@ export class Bootstrap extends EventEmitter {
         this.storage.storePerformanceStatistics as (
           performanceStatistics: Statistics
         ) => Promise<void>
-      )(data).catch(Constants.EMPTY_FUNCTION)
+      )(data).catch((error: unknown) => {
+        logger.error(`${this.logPrefix()} Error while storing performance statistics:`, error)
+      })
     } else {
       ;(this.storage?.storePerformanceStatistics as (performanceStatistics: Statistics) => void)(
         data

diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts
index db4469a3c8d8a35489ef7969007c5d32c5193615..f3df70b3c47b01c58fd39e93b1b529584b3f37b4 100644 (file)
--- a/src/charging-station/ChargingStation.ts
+++ b/src/charging-station/ChargingStation.ts
@@ -813,7 +813,9 @@ export class ChargingStation extends EventEmitter {
 
     // Handle WebSocket message
     this.wsConnection.on('message', data => {
-      this.onMessage(data).catch(Constants.EMPTY_FUNCTION)
+      this.onMessage(data).catch((error: unknown) =>
+        logger.error(`${this.logPrefix()} Error while processing WebSocket message:`, error)
+      )
     })
     // Handle WebSocket error
     this.wsConnection.on('error', this.onError.bind(this))
@@ -951,7 +953,8 @@ export class ChargingStation extends EventEmitter {
                 } else {
                   this.performanceStatistics?.stop()
                 }
-                // FIXME?: restart heartbeat and WebSocket ping when their interval values have changed
+                this.restartHeartbeat()
+                this.restartWebSocketPing()
               } catch (error) {
                 logger.error(
                   `${this.logPrefix()} ${FileType.ChargingStationTemplate} file monitoring error:`,

diff --git a/src/charging-station/ConfigurationKeyUtils.ts b/src/charging-station/ConfigurationKeyUtils.ts
index 709914b827e4e937c4b2e4bf7c7a1fae5bd9b3bf..e39791363d298ca4ab26b363642655c624db35b0 100644 (file)
--- a/src/charging-station/ConfigurationKeyUtils.ts
+++ b/src/charging-station/ConfigurationKeyUtils.ts
@@ -18,6 +18,15 @@ interface DeleteConfigurationKeyParams {
   save?: boolean
 }
 
+const matchesConfigurationKey = (
+  configElement: ConfigurationKey,
+  key: ConfigurationKeyType,
+  caseInsensitive: boolean
+): boolean =>
+  caseInsensitive
+    ? configElement.key.toLowerCase() === key.toLowerCase()
+    : configElement.key === key
+
 export const getConfigurationKey = (
   chargingStation: ChargingStation,
   key: ConfigurationKeyType,
@@ -25,9 +34,7 @@ export const getConfigurationKey = (
 ): ConfigurationKey | undefined => {
   if (!Array.isArray(chargingStation.ocppConfiguration?.configurationKey)) return undefined
   return chargingStation.ocppConfiguration.configurationKey.find(configElement =>
-    caseInsensitive
-      ? configElement.key.toLowerCase() === key.toLowerCase()
-      : configElement.key === key
+    matchesConfigurationKey(configElement, key, caseInsensitive)
   )
 }
 
@@ -40,9 +47,7 @@ const getConfigurationKeyIndex = (
     return -1
   }
   return chargingStation.ocppConfiguration.configurationKey.findIndex(configElement =>
-    caseInsensitive
-      ? configElement.key.toLowerCase() === key.toLowerCase()
-      : configElement.key === key
+    matchesConfigurationKey(configElement, key, caseInsensitive)
   )
 }
 

diff --git a/src/charging-station/Helpers.ts b/src/charging-station/Helpers.ts
index 231ddda53ab015456fdedbcea3933c12d984ba61..761d976a08af8c508a0643e5ba85354c1590b467 100644 (file)
--- a/src/charging-station/Helpers.ts
+++ b/src/charging-station/Helpers.ts
@@ -169,13 +169,20 @@ export const removeExpiredReservations = async (
       chargingStation.removeReservation(reservation, ReservationTerminationReason.EXPIRED)
     )
   )
+  let failureCount = 0
   for (const result of results) {
     if (result.status === 'rejected') {
+      ++failureCount
       logger.warn(
         `${chargingStation.logPrefix()} ${moduleName}.removeExpiredReservations: reservation removal failed: ${String(result.reason)}`
       )
     }
   }
+  if (failureCount > 0) {
+    logger.error(
+      `${chargingStation.logPrefix()} ${moduleName}.removeExpiredReservations: ${failureCount.toString()}/${reservations.length.toString()} expired reservation removal(s) failed`
+    )
+  }
 }
 
 export const getNumberOfReservableConnectors = (
@@ -993,13 +1000,13 @@ const convertDeprecatedTemplateKey = (
   deprecatedKey: string,
   key?: string
 ): void => {
-  if (template[deprecatedKey as keyof ChargingStationTemplate] != null) {
+  const templateRecord = template as unknown as Record<string, unknown>
+  if (templateRecord[deprecatedKey] != null) {
     if (key != null) {
-      ;(template as unknown as Record<string, unknown>)[key] =
-        template[deprecatedKey as keyof ChargingStationTemplate]
+      templateRecord[key] = templateRecord[deprecatedKey]
     }
     // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
-    delete template[deprecatedKey as keyof ChargingStationTemplate]
+    delete templateRecord[deprecatedKey]
   }
 }
 
@@ -1218,10 +1225,28 @@ export const prepareChargingProfileKind = (
         )
         delete chargingSchedule.startSchedule
       }
-      if (connectorStatus?.transactionStarted === true) {
-        chargingSchedule.startSchedule = connectorStatus.transactionStart
+      if (connectorStatus?.transactionStarted !== true) {
+        logger.debug(
+          `${logPrefix} ${moduleName}.prepareChargingProfileKind: Relative charging profile id ${chargingProfileId} has no active transaction, cannot be evaluated`
+        )
+        return false
+      }
+      chargingSchedule.startSchedule = connectorStatus.transactionStart
+      if (chargingSchedule.startSchedule == null) {
+        logger.warn(
+          `${logPrefix} ${moduleName}.prepareChargingProfileKind: Relative charging profile id ${chargingProfileId} has active transaction without start date`
+        )
+        return false
+      }
+      if (chargingSchedule.duration != null) {
+        const elapsedSeconds = differenceInSeconds(currentDate, chargingSchedule.startSchedule)
+        if (elapsedSeconds > chargingSchedule.duration) {
+          logger.debug(
+            `${logPrefix} ${moduleName}.prepareChargingProfileKind: Relative charging profile id ${chargingProfileId} duration ${chargingSchedule.duration.toString()}s exceeded (elapsed: ${elapsedSeconds.toString()}s)`
+          )
+          return false
+        }
       }
-      // FIXME: handle relative charging profile duration
       break
   }
   return true

diff --git a/src/charging-station/ui-server/AbstractUIServer.ts b/src/charging-station/ui-server/AbstractUIServer.ts
index 20bf50fce579e919e22d5e15a064c77e350b93c4..3505f65f600e92cda38621f11dc69878c65d68b5 100644 (file)
--- a/src/charging-station/ui-server/AbstractUIServer.ts
+++ b/src/charging-station/ui-server/AbstractUIServer.ts
@@ -20,7 +20,7 @@ import {
   type UIServerConfiguration,
   type UUIDv4,
 } from '../../types/index.js'
-import { isEmpty, logger } from '../../utils/index.js'
+import { isEmpty, isNotEmptyString, logger, logPrefix } from '../../utils/index.js'
 import { UIServiceFactory } from './ui-services/UIServiceFactory.js'
 import { isValidCredential } from './UIServerSecurity.js'
 import { getUsernameAndPasswordFromAuthorizationToken } from './UIServerUtils.js'
@@ -31,6 +31,8 @@ export abstract class AbstractUIServer {
   protected readonly httpServer: Http2Server | Server
   protected readonly responseHandlers: Map<UUIDv4, ServerResponse | WebSocket>
 
+  protected abstract readonly uiServerType: string
+
   protected readonly uiServices: Map<ProtocolVersion, AbstractUIService>
 
   private readonly chargingStations: Map<string, ChargingStationData>
@@ -105,7 +107,15 @@ export abstract class AbstractUIServer {
     return [...this.chargingStations.values()]
   }
 
-  public abstract logPrefix (moduleName?: string, methodName?: string, prefixSuffix?: string): string
+  public logPrefix = (modName?: string, methodName?: string, prefixSuffix?: string): string => {
+    const logMsgPrefix =
+      prefixSuffix != null ? `${this.uiServerType} ${prefixSuffix}` : this.uiServerType
+    const logMsg =
+      isNotEmptyString(modName) && isNotEmptyString(methodName)
+        ? ` ${logMsgPrefix} | ${modName}.${methodName}:`
+        : ` ${logMsgPrefix} |`
+    return logPrefix(logMsg)
+  }
 
   public async sendInternalRequest (request: ProtocolRequest): Promise<ProtocolResponse> {
     const protocolVersion = ProtocolVersion['0.0.1']

diff --git a/src/charging-station/ui-server/UIHttpServer.ts b/src/charging-station/ui-server/UIHttpServer.ts
index 7b50c1952d04ac383fb422f92e47675b6bde434b..cf6fee380bf48bf3e8578e9b03a6b7608cc3f298 100644 (file)
--- a/src/charging-station/ui-server/UIHttpServer.ts
+++ b/src/charging-station/ui-server/UIHttpServer.ts
@@ -17,13 +17,7 @@ import {
   type UIServerConfiguration,
   type UUIDv4,
 } from '../../types/index.js'
-import {
-  generateUUID,
-  isNotEmptyString,
-  JSONStringify,
-  logger,
-  logPrefix,
-} from '../../utils/index.js'
+import { generateUUID, JSONStringify, logger } from '../../utils/index.js'
 import { AbstractUIServer } from './AbstractUIServer.js'
 import {
   createBodySizeLimiter,
@@ -47,6 +41,8 @@ enum HttpMethods {
 }
 
 export class UIHttpServer extends AbstractUIServer {
+  protected override readonly uiServerType = 'UI HTTP Server'
+
   private readonly acceptsGzip: Map<UUIDv4, boolean>
 
   public constructor (protected override readonly uiServerConfiguration: UIServerConfiguration) {
@@ -54,15 +50,6 @@ export class UIHttpServer extends AbstractUIServer {
     this.acceptsGzip = new Map<UUIDv4, boolean>()
   }
 
-  public logPrefix = (modName?: string, methodName?: string, prefixSuffix?: string): string => {
-    const logMsgPrefix = prefixSuffix != null ? `UI HTTP Server ${prefixSuffix}` : 'UI HTTP Server'
-    const logMsg =
-      isNotEmptyString(modName) && isNotEmptyString(methodName)
-        ? ` ${logMsgPrefix} | ${modName}.${methodName}:`
-        : ` ${logMsgPrefix} |`
-    return logPrefix(logMsg)
-  }
-
   public sendRequest (request: ProtocolRequest): void {
     switch (this.uiServerConfiguration.version) {
       case ApplicationProtocolVersion.VERSION_20:

diff --git a/src/charging-station/ui-server/UIServerFactory.ts b/src/charging-station/ui-server/UIServerFactory.ts
index 3bdf68f6641688ec55dee124e9fee0abc2a6c182..b0c87bba778b5963337d3f135793bb97f018b477 100644 (file)
--- a/src/charging-station/ui-server/UIServerFactory.ts
+++ b/src/charging-station/ui-server/UIServerFactory.ts
@@ -1,5 +1,3 @@
-import chalk from 'chalk'
-
 import type { AbstractUIServer } from './AbstractUIServer.js'
 
 import { BaseError } from '../../exception/index.js'
@@ -48,7 +46,6 @@ export class UIServerFactory {
     ) {
       const logMsg = `Non loopback address in '${ConfigurationSection.uiServer}' configuration section without authentication enabled. This is not recommended`
       logger.warn(`${UIServerFactory.logPrefix()} ${logMsg}`)
-      console.warn(chalk.yellow(logMsg))
     }
     if (
       uiServerConfiguration.type === ApplicationProtocol.WS &&
@@ -56,7 +53,6 @@ export class UIServerFactory {
     ) {
       const logMsg = `Only version ${ApplicationProtocolVersion.VERSION_11} with application protocol type '${uiServerConfiguration.type}' is supported in '${ConfigurationSection.uiServer}' configuration section. Falling back to version ${ApplicationProtocolVersion.VERSION_11}`
       logger.warn(`${UIServerFactory.logPrefix()} ${logMsg}`)
-      console.warn(chalk.yellow(logMsg))
       uiServerConfiguration.version = ApplicationProtocolVersion.VERSION_11
     }
     switch (uiServerConfiguration.type) {
@@ -77,7 +73,6 @@ export class UIServerFactory {
             ApplicationProtocol.WS
           }'`
           logger.warn(`${UIServerFactory.logPrefix()} ${logMsg}`)
-          console.warn(logMsg)
         }
         return new UIWebSocketServer(uiServerConfiguration)
     }

diff --git a/src/charging-station/ui-server/UIServerSecurity.ts b/src/charging-station/ui-server/UIServerSecurity.ts
index 306a0644490e238d11f73adf173a56142742f9f2..6295cf54894964418c581d7749508b43a5ece822 100644 (file)
--- a/src/charging-station/ui-server/UIServerSecurity.ts
+++ b/src/charging-station/ui-server/UIServerSecurity.ts
@@ -27,6 +27,7 @@ export const isValidCredential = (provided: string, expected: string): boolean =
 
     return timingSafeEqual(paddedProvided, paddedExpected)
   } catch {
+    // Intentional: fail closed on any comparison error (e.g., encoding issues)
     return false
   }
 }

diff --git a/src/charging-station/ui-server/UIWebSocketServer.ts b/src/charging-station/ui-server/UIWebSocketServer.ts
index d42cf798490398548c03583a67e811c39d60e296..332d27ff6b6a661a3c3771788d4bbd94859e1773 100644 (file)
--- a/src/charging-station/ui-server/UIWebSocketServer.ts
+++ b/src/charging-station/ui-server/UIWebSocketServer.ts
@@ -13,10 +13,8 @@ import {
 } from '../../types/index.js'
 import {
   getWebSocketCloseEventStatusString,
-  isNotEmptyString,
   JSONStringify,
   logger,
-  logPrefix,
   validateUUID,
 } from '../../utils/index.js'
 import { AbstractUIServer } from './AbstractUIServer.js'
@@ -30,6 +28,8 @@ import {
 const moduleName = 'UIWebSocketServer'
 
 export class UIWebSocketServer extends AbstractUIServer {
+  protected override readonly uiServerType = 'UI WebSocket Server'
+
   private readonly webSocketServer: WebSocketServer
 
   public constructor (protected override readonly uiServerConfiguration: UIServerConfiguration) {
@@ -56,16 +56,6 @@ export class UIWebSocketServer extends AbstractUIServer {
     })
   }
 
-  public logPrefix = (modName?: string, methodName?: string, prefixSuffix?: string): string => {
-    const logMsgPrefix =
-      prefixSuffix != null ? `UI WebSocket Server ${prefixSuffix}` : 'UI WebSocket Server'
-    const logMsg =
-      isNotEmptyString(modName) && isNotEmptyString(methodName)
-        ? ` ${logMsgPrefix} | ${modName}.${methodName}:`
-        : ` ${logMsgPrefix} |`
-    return logPrefix(logMsg)
-  }
-
   public sendRequest (request: ProtocolRequest): void {
     this.broadcastToClients(JSON.stringify(request))
   }