From 44c81e8a5873a8676fc8dc5893cc2ebeb3076534 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Wed, 30 Sep 2026 20:58:15 +0200 Subject: [PATCH] fix(ui-server): validate UI request payloads against a canonical schema (#2129) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * fix(ui-server): validate UI request payloads against a canonical schema The three UI transports only guaranteed the frame shape, never the payload fields: WebSocket checked the SRPC tuple, HTTP only parsed JSON, and MCP was the sole path with a per-procedure schema. Enforcement is now a single gate in AbstractUIService.requestHandler, so all three transports share one contract. Closes a targeting escalation: a non-array `hashIds` failed the `Array.isArray` guard in sendBroadcastChannelRequest and therefore degraded into a broadcast to every station. Also removes the unchecked `deleteConfiguration as boolean` cast in the worker broadcast channel and the now-duplicated payload type check in handleAddChargingStations. Schemas are loose objects: OCPP PDU fields are forwarded untouched and stay validated by the OCPP layer against the OCPP JSON schemas. Coverage is exhaustive by construction (Record). Verified: 3961 tests, 0 fail; typecheck, lint and build green. The five new regression tests fail on the previous commit. * chore(serena): refresh project configuration Written by the Serena tooling on a newer version: refreshed the language-server list (deno, gleam, nextflow, wolfram, julia_fatou), added included_apis / excluded_apis / agent_interface keys, and updated the special-requirements notes. No functional change to the simulator. * fix(ui-server): require the connector target the worker already requires Review follow-up on the payload gate. START_TRANSACTION and STATUS_NOTIFICATION were still treating connectorId as an unvalidated loose PDU field, while handleStartTransaction and handleStatusNotification reject a missing one with a per-station failure. A non-numeric value passed their == null guard and reached the station. Also aligns setSupervisionUrl with the MCP tool contract, which has always required a real URL, and makes stopTransaction require transactionId - the field handleStopTransaction actually reads - instead of declaring a connectorId it ignores. Five regression tests added; suite is 3963 tests, 0 fail. * fix(ui-server): honour the OCPP connector/EVSE 0 targets in the payload gate Review round 1. Two identifier semantics, on specification evidence: OCPP 1.6 6.31 (MeterValues) and 6.47 (StatusNotification) define connectorId >= 0 with 0 designating the main power meter / charging station main controller, 6.45 (StartTransaction) requires connectorId > 0, and OCPP 2.0.1 MeterValuesRequest reserves evseId 0 for the main power meter. A single positive() constraint was rejecting targets the simulator materialises by default (OCPPConnectorStatusOperations.ts:187, OCPPServiceUtils.ts:1420, connector "0" in every template). Split into connectorIdField / evseIdField (non-negative) and physicalConnectorIdField (positive) for the cable lock and transaction connectors. Also founded by the review: - supervisionUser is now the same RFC 7617 field everywhere; accepting a colon on setSupervisionUrl made openWSConnection silently drop the auth. - stopTransaction transactionId is integer, per StopTransaction.json; the string branch was unreachable on a 1.6-only procedure. - Error amplification capped: an invalid array produced one issue per element, a 60k-element hashIds yielding a 3.8 MB errorMessage. Now 626 chars. - evseId, the connectorStatus/status pair and meterValue[].sampledValue are declared, closing the gaps left by the worker-side guards. - Module header corrected: UIMCPServer flattens the OCPP payload, so all three transports observe identical behaviour - only the wording overstated it. - isEmpty for the issue path, dead duplicate schema removed, unused exports dropped, barrel inventory updated, shared test helper, 8 tests added (13->21). README states the bounds it left implicit. * fix(ui-server): group payload violations per field and truly share the schemas Round 2 review. Three defects were introduced by the previous fix commit. The 10-issue cap dropped whole fields: 12 invalid options plus a non-string template produced a message that never mentioned template, so the client discovered it only on the next round trip. formatIssues now groups by top-level field and reports the first violation plus a count; array indices collapse into the group key. A 60000-element hashIds now yields a 79-character message and every distinct field stays visible. The 'shared' supervision fields were declared as unexported consts while MCPToolSchemas still duplicated z.url(), the RFC 7617 regex and z.string() verbatim, under a header claiming a field cannot mean two things. The three primitives are now exported and imported; no literal duplication remains. stopTransaction declared an evseId the worker never reads (ChargingStationWorkerBroadcastChannel.ts:791-825), rejecting payloads for a field the server ignores. Removed. Also: transactionId is required in both modules; errorCode is declared optional because 2.0.1 does not carry it and the gate is version-blind; the false '2.0.x sends evseId instead' comment is corrected; the OCPP 1.6 connectorId contradiction is recorded in the JSDoc; README gains the missing meterValues section. Tests 21 -> 27, each pinning a defect that previously survived mutation. * fix(ocpp): resolve the 1.6 status alias and unblock the published MCP contract Round 3 review. Two P1 regressions introduced by the previous commit. buildStatusNotificationRequest copied only commandParams.status, so a 2.0.x-shaped payload produced a 1.6 PDU with status: undefined and AJV rejected it per station. It now resolves connectorStatus ?? status, matching OCPP20ServiceUtils:1292 and OCPPConnectorStatusOperations.ts:44, and refuses out-of-enum values explicitly: Occupied is the only 2.0.1 status with no 1.6 counterpart, and a bare comment would have let it reach AJV again. This predates the PR - the worker guard at :783 always accepted connectorStatus - but the gate made it contractual and a test enshrined it as correct. The MCP tool schema required transactionId at the top level while the server publishes ocpp16Payload as StopTransaction.json, whose required list places transactionId inside the payload. The SDK validates before the handler, so a client following the advertised contract was rejected with -32602. Back to optional: z.object strips undeclared keys silently, so removing the property would have been worse. Also: violations are grouped by the full normalized path, so nested sub-fields stay visible and counted issues are no longer attributed to another path; meterValues requires at least one of connectorId/evseId, the union the version-blind gate can express and the worker's own condition; the module header now states the layer split truthfully. Tests 27 -> 33, plus 4 on the 1.6 builder. * fix(ui-server): preserve MCP passthrough and unblock the web supervision form Round 4 review. The PR body claimed all three transports observe identical behaviour; they did not. UIMCPServer registered the tool schema as a bare shape, so the SDK rebuilt it through objectFromShape in strip mode and the handler received the payload stripped of undeclared top-level fields. Fixing it needs BOTH halves - build the tool schemas as looseObject AND register the object rather than its shape; either alone still strips. Verified as an invariant over all 36 tool schemas: whenever a parse succeeds, an undeclared top-level field survives. The RFC 7617 rule on supervisionUser, extended from the MCP transport to the shared gate, broke the shipped web UI: the modern form pre-fills the field from the station and sends it unconditionally, so a station whose template holds a colon in supervisionUser could no longer change its supervision URL - a payload main accepted and the client structurally cannot omit. The form now omits credentials left at their base value; the classic skin, starting empty, still sends an empty field and keeps the documented clear-the-value meaning. Also: the module header made three claims the code contradicts (it excluded PDU members the gate does validate, called every schema loose, and described transactionId as optional 250 lines below its own requirement), evseId's zero semantics were attributed to statusNotification instead of meterValues, and meterValue[].sampledValue optionality was untested - removing the .optional() left the suite green. * fix(ui-web): correct the supervision credentials hint The hint still said credentials are sent verbatim and that leaving a field empty clears the stored value. Since the form now omits credentials left at their base, an emptied field that was never edited no longer reaches the server, and a username containing ':' is rejected (RFC 7617). The text has to describe the behaviour the form actually has. * fix(ui-server): drop three bounds no specification or worker requires Every remaining rejection has to name the rule that mandates it. Three did not. lockConnector: the message does not exist in the OCPP 1.6 core document (grep -c returns 0) and ships no JSON schema - it is an OCA addendum. Only StartTransaction (§6.45) and UnlockConnector (§6.53) declare connectorId > 0, so those two keep the bound and lockConnector returns to the non-negative integer the project already accepted. The JSDoc now cites a section per procedure instead of §6.45 for three. connectorIds: the worker strips it from every command except the two automatic transaction generator ones (ChargingStationWorkerBroadcastChannel.cleanRequestPayload), so the gate was rejecting on 27 procedures a payload the server already accepted and then discarded. It is now declared only where it is read. supervisionUser in a station option: TemplateSchema accepts any string and ChargingStation.openWSConnection deliberately degrades a colon-bearing user to a warning with the auth omitted. Bounding the option more strictly than the template made one configuration value load from a file but be rejected over the API. The strict rule stays on SET_SUPERVISION_URL, where the operator types the value, and on the web form, which now omits unchanged credentials. Also corrects the OCPP 2.0.1 section for MeterValuesRequest (§1.32.1, not §1.31.1, which is LogStatusNotificationRequest) and names supervisionUser/supervisionPassword in the module scope header. Two tests pin the new acceptances. * docs(ui-server): correct OCPP and MCP validation semantics * fix(web): reject colon-bearing supervision usernames before submission * fix(ui-server): redact credentials from failure diagnostics * docs: clarify UI validation rationale and trim redundant comments * docs(web): remove supervision URL form section * docs: remove redundant UI payload validation prose * fix(ocpp16): tighten status notification builder contract * fix(ui-server): require sampled values before meter broadcast --- .serena/project.yml | 42 +- README.md | 42 +- .../ocpp/1.6/OCPP16RequestService.ts | 4 +- .../ocpp/1.6/OCPP16ServiceUtils.ts | 37 +- src/charging-station/ui-server/UIMCPServer.ts | 4 +- src/charging-station/ui-server/index.ts | 2 + .../ui-server/mcp/MCPToolSchemas.ts | 96 +- .../ui-services/AbstractUIService.ts | 57 +- .../UIServiceRequestPayloadSchemas.ts | 334 ++++++ src/types/index.ts | 1 + src/types/ocpp/1.6/Requests.ts | 10 + .../ocpp/1.6/OCPP16ServiceUtils.test.ts | 45 +- .../ui-server/UIServerTestUtils.ts | 26 +- .../ui-services/AbstractUIService.test.ts | 16 +- .../UIServiceRequestPayloadSchemas.test.ts | 973 ++++++++++++++++++ .../src/shared/composables/useSetUrlForm.ts | 39 +- .../dialogs/SetSupervisionUrlDialog.vue | 15 +- .../shared/composables/useSetUrlForm.test.ts | 94 +- 18 files changed, 1664 insertions(+), 173 deletions(-) create mode 100644 src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts create mode 100644 tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts diff --git a/.serena/project.yml b/.serena/project.yml index 92bec931..4ec0939a 100644 --- a/.serena/project.yml +++ b/.serena/project.yml @@ -143,18 +143,19 @@ activation_command_timeout: 180.0 # list of language servers to start when using the LSP backend; choose from: # ada al angular ansible bash # bsl clojure cpp cpp_ccls crystal -# csharp csharp_omnisharp cue dart elixir -# elm erlang fortran fsharp gdscript -# go groovy haskell haxe hlsl -# html java json julia kotlin -# latex lean4 lua luau markdown -# matlab msl nix ocaml pascal -# perl php php_phpactor php_phpantom powershell -# python python_basedpyright python_jedi python_pyrefly python_ty -# qml r rego ruby ruby_solargraph -# rust scala scss solidity svelte -# swift systemverilog terraform toml typescript -# typescript_vts vue yaml zig +# csharp csharp_omnisharp cue dart deno +# elixir elm erlang fortran fsharp +# gdscript gleam go groovy haskell +# haxe hlsl html java json +# julia julia_fatou kotlin latex lean4 +# lua luau markdown matlab msl +# nextflow nix ocaml pascal perl +# php php_phpactor php_phpantom powershell python +# python_basedpyright python_jedi python_pyrefly python_ty qml +# r rego ruby ruby_solargraph rust +# scala scss solidity svelte swift +# systemverilog terraform toml typescript typescript_vts +# vue wolfram yaml zig # (This list may be outdated; generated with scripts/print_language_list.py; # For the current list, see values of the LanguageServerId enum here: # https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py) @@ -164,8 +165,11 @@ activation_command_timeout: 180.0 # - For JavaScript, use typescript # - For Angular projects, use angular (subsumes typescript+html; requires `npm install` in the project root) # - For Svelte projects, use svelte (subsumes typescript/javascript for .svelte projects; requires npm) +# - For Deno projects, use deno (serves the same .ts/.js files as typescript; requires the deno CLI on PATH) # - For SCSS / Sass / plain CSS, use scss (some-sass-language-server handles all three) # - For Free Pascal/Lazarus, use pascal +# - External Python adapters may add further registered IDs; install the adapter package first +# and then use its ID here, for example: example # Special requirements: # Some language servers require additional setup/installations. # See here for details: https://oraios.github.io/serena/01-about/020_programming-languages.html#language-servers @@ -174,3 +178,17 @@ activation_command_timeout: 180.0 # Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored. language_servers: - typescript + +# list of APIs (facades or facade methods, e.g. "lsp" or "lsp.get_diagnostics_for_symbol") to include in the REPL +# that would otherwise be disabled (particularly optional methods, which are disabled by default). +# This extends the existing inclusions (e.g. from the global configuration). +included_apis: [] + +# list of APIs (facades or facade methods, e.g. "lsp" or "lsp.find_symbol") to exclude from the REPL. +# This extends the existing exclusions (e.g. from the global configuration). +excluded_apis: [] + +# The interface through which the agent (LLM) accesses Serena's functionality (overrides the global setting). +# Valid values: tools, REPL (see the global configuration for details); leave empty to use the global setting. +# Note: the interface is fixed at startup. If a project is activated post-init, its setting is not applied. +agent_interface: diff --git a/README.md b/README.md index fa39cdb4..d2e7af12 100644 --- a/README.md +++ b/README.md @@ -1125,11 +1125,11 @@ Set the WebSocket header _Sec-WebSocket-Protocol_ to `ui0.0.1`. `ProcedureName`: 'setSupervisionUrl' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `url`: string, + `url`: absolute URL string, `supervisionUser?`: string, `supervisionPassword?`: string } - `url` is required. `supervisionUser` and `supervisionPassword` are each optional and independent: a string (including `""`, which clears the field) updates the value; omitting the field preserves the existing value. Changes take effect on the next WebSocket (re)connect. + `url` is required and must be an absolute URL. `supervisionUser` must not contain `:` (RFC 7617). `supervisionUser` and `supervisionPassword` are each optional and independent: a string (including `""`, which clears the field) updates the value; omitting the field preserves the existing value. Changes take effect on the next WebSocket (re)connect. - Response: `PDU`: { @@ -1252,7 +1252,7 @@ Set the WebSocket header _Sec-WebSocket-Protocol_ to `ui0.0.1`. `ProcedureName`: 'startAutomaticTransactionGenerator' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `connectorIds`: connector id integer array (optional, default: all connectors) + `connectorIds`: physical connector id integer array (>= 1, optional, default: all connectors) } - Response: @@ -1269,7 +1269,7 @@ Set the WebSocket header _Sec-WebSocket-Protocol_ to `ui0.0.1`. `ProcedureName`: 'stopAutomaticTransactionGenerator' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `connectorIds`: connector id integer array (optional, default: all connectors) + `connectorIds`: physical connector id integer array (>= 1, optional, default: all connectors) } - Response: @@ -1286,7 +1286,7 @@ Set the WebSocket header _Sec-WebSocket-Protocol_ to `ui0.0.1`. `ProcedureName`: 'lockConnector' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `connectorId`: connector id integer + `connectorId`: connector id integer (>= 0; 0 is the main controller, which holds no cable lock) } - Response: @@ -1303,7 +1303,7 @@ Set the WebSocket header _Sec-WebSocket-Protocol_ to `ui0.0.1`. `ProcedureName`: 'unlockConnector' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `connectorId`: connector id integer + `connectorId`: physical connector id integer (>= 1) } - Response: @@ -1355,13 +1355,30 @@ Examples: `responsesFailed`: failed responses payload array (optional) } +- **Meter Values** + - Request: + `ProcedureName`: 'meterValues' + `PDU`: { + `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), + `connectorId?` or `evseId?`: connector or EVSE identifier integer (>= 0; 0 designates the main power meter); at least one is required, + `meterValue?`: array of meter value objects, each with a required `sampledValue` array; omit to use the station's current values + } + + - Response: + `PDU`: { + `status`: 'success' | 'failure', + `hashIdsSucceeded`: charging station unique identifier strings array, + `hashIdsFailed`: charging station unique identifier strings array (optional), + `responsesFailed`: failed responses payload array (optional) + } + - **Start Transaction** - Request: `ProcedureName`: 'startTransaction' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `connectorId`: connector id integer, - `idTag`: RFID tag string + `connectorId`: physical connector id integer (>= 1), + `idTag?`: RFID tag string (optional, defaults to `'00000000'`) } - Response: @@ -1380,6 +1397,8 @@ Examples: `transactionId`: transaction id integer } + The connector is resolved from the transaction, hence no connector identifier is sent. + - Response: `PDU`: { `status`: 'success' | 'failure', @@ -1393,9 +1412,10 @@ Examples: `ProcedureName`: 'statusNotification' `PDU`: { `hashIds`: charging station unique identifier strings array (optional, default: all charging stations), - `connectorId`: connector id integer, - `errorCode`: connector error code, - `status`: connector status + `connectorId`: connector id integer (>= 0, 0 designates the charging station main controller), + `evseId?`: EVSE id integer (>= 0), + `errorCode?`: connector error code (optional, absent from the OCPP 2.0.1 request), + `status` or `connectorStatus`: connector status } - Response: diff --git a/src/charging-station/ocpp/1.6/OCPP16RequestService.ts b/src/charging-station/ocpp/1.6/OCPP16RequestService.ts index 8013686b..e1ff82f9 100644 --- a/src/charging-station/ocpp/1.6/OCPP16RequestService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16RequestService.ts @@ -12,7 +12,7 @@ import { type OCPP16MeterValue, OCPP16RequestCommand, type OCPP16StartTransactionRequest, - type OCPP16StatusNotificationRequest, + type OCPP16StatusNotificationRequestParams, OCPPVersion, } from '../../../types/index.js' import { assertIsJsonObject, Constants, logger } from '../../../utils/index.js' @@ -140,7 +140,7 @@ export class OCPP16RequestService extends OCPPRequestService { return OCPP16ServiceUtils.buildStatusNotificationRequest({ errorCode: ChargePointErrorCode.NO_ERROR, ...params, - } as OCPP16StatusNotificationRequest) + } as OCPP16StatusNotificationRequestParams) case OCPP16RequestCommand.STOP_TRANSACTION: { const transactionId = params.transactionId as number const hasIdTag = Object.hasOwn(params, 'idTag') diff --git a/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts b/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts index 9d8b5c90..fc5a49f8 100644 --- a/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts +++ b/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts @@ -53,6 +53,7 @@ import { type OCPP16SignedMeterValue, OCPP16StandardParametersKey, type OCPP16StatusNotificationRequest, + type OCPP16StatusNotificationRequestParams, OCPP16StopTransactionReason, type OCPP16SupportedFeatureProfiles, OCPP16VendorParametersKey, @@ -110,6 +111,18 @@ const moduleName = 'OCPP16ServiceUtils' const RFC3339_TIMESTAMP_PATTERN = /^(\d{4})-(\d{2})-(\d{2})[Tt](\d{2}):(\d{2}):(\d{2})(?:\.(\d+))?([Zz]|([+-])(\d{2}):(\d{2}))$/ const DAYS_PER_MONTH = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] as const +const OCPP16_CHARGE_POINT_STATUSES: ReadonlySet = new Set( + Object.values(OCPP16ChargePointStatus) +) + +/** + * Rejects statuses that OCPP 1.6 cannot encode: OCPP 2.0.1 `Occupied` has no + * 1.6 counterpart (OCPP 2.0.1 Part 2 §3.23). + * @param status - Untrusted connector status. + * @returns `true` when the value is an OCPP 1.6 charge point status. + */ +const isOCPP16ChargePointStatus = (status: string): status is OCPP16ChargePointStatus => + OCPP16_CHARGE_POINT_STATUSES.has(status) const isLeapYear = (year: number): boolean => year % 4 === 0 && (year % 100 !== 0 || year % 400 === 0) @@ -290,16 +303,30 @@ export class OCPP16ServiceUtils { } /** - * @param commandParams - Status notification parameters + * @param commandParams - Status notification parameters; `connectorStatus` + * takes precedence over `status` * @returns Formatted OCPP 1.6 StatusNotification request payload + * @throws {OCPPError} When no connector status is supplied, or when it is not + * a valid OCPP 1.6 charge point status */ public static buildStatusNotificationRequest ( - commandParams: OCPP16StatusNotificationRequest + commandParams: OCPP16StatusNotificationRequestParams ): OCPP16StatusNotificationRequest { + const { connectorId, errorCode } = commandParams + const status = commandParams.connectorStatus ?? commandParams.status + if (status == null || !isOCPP16ChargePointStatus(status)) { + throw new OCPPError( + ErrorType.INTERNAL_ERROR, + `Cannot build status notification payload: invalid connector status for connector ${connectorId.toString()}`, + RequestCommand.STATUS_NOTIFICATION + ) + } return { - connectorId: commandParams.connectorId, - errorCode: commandParams.errorCode, - status: commandParams.status, + connectorId, + // OCPP 2.0.1 has no errorCode; the 1.6 request service supplies NO_ERROR + // before calling this builder. + errorCode, + status, } satisfies OCPP16StatusNotificationRequest } diff --git a/src/charging-station/ui-server/UIMCPServer.ts b/src/charging-station/ui-server/UIMCPServer.ts index bde72eac..145a6beb 100644 --- a/src/charging-station/ui-server/UIMCPServer.ts +++ b/src/charging-station/ui-server/UIMCPServer.ts @@ -225,7 +225,9 @@ export class UIMCPServer extends AbstractUIServer { procedureName, { description: schema.description, - inputSchema: schema.inputSchema.shape, + // Register the loose object itself: the SDK rebuilds raw shapes as + // stripping objects, which would drop unlisted flat OCPP fields. + inputSchema: schema.inputSchema, }, async (input: Record) => { return await this.invokeProcedure(procedureName, input as RequestPayload, this.service) diff --git a/src/charging-station/ui-server/index.ts b/src/charging-station/ui-server/index.ts index 2d240f8d..03cdcdc2 100644 --- a/src/charging-station/ui-server/index.ts +++ b/src/charging-station/ui-server/index.ts @@ -29,6 +29,8 @@ * transport-agnostic enum surfaced through this barrel. * - `ui-services/UIService001` / `ui-services/UIServiceFactory` concrete * implementations — internal to `AbstractUIService`. + * - `ui-services/UIServiceRequestPayloadSchemas` — shared internally by + * the request gate and MCP tool schemas; not exported. */ export type { AbstractUIServer } from './AbstractUIServer.js' export { diff --git a/src/charging-station/ui-server/mcp/MCPToolSchemas.ts b/src/charging-station/ui-server/mcp/MCPToolSchemas.ts index be14ab5a..7fb07094 100644 --- a/src/charging-station/ui-server/mcp/MCPToolSchemas.ts +++ b/src/charging-station/ui-server/mcp/MCPToolSchemas.ts @@ -1,78 +1,32 @@ import { z } from 'zod' import { ProcedureName } from '../../../types/index.js' +import { + chargingStationOptionsSchema, + connectorIdsField as connectorIds, + hashIdsField as hashIds, + physicalConnectorIdField, + supervisionPasswordField, + supervisionUserField, + urlField, +} from '../ui-services/UIServiceRequestPayloadSchemas.js' export interface MCPToolSchema { description: string inputSchema: z.ZodObject } -const hashIds = z - .array(z.string()) - .optional() - .describe('Target station hash IDs (omit for all stations)') - -const connectorIds = z - .array(z.number().int().positive()) - .optional() - .describe('Target connector IDs') - -const broadcastInputSchema = z.object({ +const broadcastInputSchema = z.looseObject({ connectorIds, hashIds, }) -const connectorInputSchema = z.object({ - connectorId: z.number().int().positive().describe('Target connector ID'), +const connectorInputSchema = z.looseObject({ + connectorId: physicalConnectorIdField, hashIds, }) -const emptyInputSchema = z.object({}) - -const chargingStationOptionsSchema = z.object({ - autoRegister: z.boolean().optional().describe('Set stations as registered at boot notification'), - autoStart: z.boolean().optional().describe('Enable automatic start of added charging station'), - baseName: z - .string() - .optional() - .describe('Override the template base name used to derive the charging station id'), - enableStatistics: z.boolean().optional().describe('Enable charging station statistics'), - fixedName: z - .boolean() - .optional() - .describe('Use base name verbatim as charging station id instead of appending index/suffix'), - nameSuffix: z - .string() - .optional() - .describe( - 'Suffix appended to the derived charging station id (ignored when fixed name is true)' - ), - ocppStrictCompliance: z - .boolean() - .optional() - .describe('Enable strict OCPP specifications adherence'), - persistentConfiguration: z - .boolean() - .optional() - .describe('Enable persistent OCPP parameters storage'), - stopTransactionsOnStopped: z - .boolean() - .optional() - .describe('Enable stop transactions on station stop'), - supervisionPassword: z - .string() - .optional() - .describe('CSMS basic auth password used on the supervision WebSocket'), - supervisionUrls: z - .union([z.url(), z.array(z.url())]) - .optional() - .describe('OCPP server supervision URL(s)'), - supervisionUser: z - .string() - .regex(/^[^:]*$/, 'must not contain ":"') - .optional() - .describe('CSMS basic auth user used on the supervision WebSocket'), -}) +const emptyInputSchema = z.looseObject({}) /** Maps ProcedureName to OCPP JSON Schema file base names per version */ export const ocppSchemaMapping = new Map([ @@ -122,7 +76,7 @@ const buildOcppInputSchema = (mapping: { ocpp16?: string; ocpp20?: string }): z. if (mapping.ocpp20 != null) { fields.ocpp20Payload = ocpp20PayloadField } - return z.object(fields) + return z.looseObject(fields) } const buildVersionAffinity = (mapping: { ocpp16?: string; ocpp20?: string }): string => { @@ -151,7 +105,7 @@ export const mcpToolSchemas = new Map([ ProcedureName.ADD_CHARGING_STATIONS, { description: 'Add new charging stations from a configuration template', - inputSchema: z.object({ + inputSchema: z.looseObject({ numberOfStations: z .number() .int() @@ -186,7 +140,7 @@ export const mcpToolSchemas = new Map([ { description: 'Change the value of an OCPP configuration key for one or more charging stations, applying the OCPP spec side effects (read-only keys are rejected)', - inputSchema: z.object({ + inputSchema: z.looseObject({ hashIds, key: z.string().min(1).describe('The OCPP configuration key to change'), value: z.string().describe('The new value to set for the configuration key'), @@ -212,7 +166,7 @@ export const mcpToolSchemas = new Map([ ProcedureName.DELETE_CHARGING_STATIONS, { description: 'Delete one or more charging stations from the simulator', - inputSchema: z.object({ + inputSchema: z.looseObject({ deleteConfiguration: z .boolean() .optional() @@ -354,18 +308,15 @@ export const mcpToolSchemas = new Map([ { description: 'Set the OCPP server supervision URL and optionally the CSMS basic auth credentials for one or more charging stations', - inputSchema: z.object({ + inputSchema: z.looseObject({ hashIds, - supervisionPassword: z - .string() + supervisionPassword: supervisionPasswordField .optional() .describe('CSMS basic auth password used on the supervision WebSocket'), - supervisionUser: z - .string() - .regex(/^[^:]*$/, 'must not contain ":"') + supervisionUser: supervisionUserField .optional() .describe('CSMS basic auth user used on the supervision WebSocket'), - url: z.url().describe('The OCPP server supervision URL to set'), + url: urlField.describe('The OCPP server supervision URL to set'), }), }, ], @@ -447,12 +398,15 @@ export const mcpToolSchemas = new Map([ ProcedureName.STOP_TRANSACTION, { description: ocppDescription('Stop a charging transaction', ProcedureName.STOP_TRANSACTION), - inputSchema: z.object({ + inputSchema: z.looseObject({ hashIds, ocpp16Payload: z .record(z.string(), z.unknown()) .optional() .describe('OCPP 1.6 StopTransaction payload'), + // The published 1.6 schema carries transactionId inside ocpp16Payload. + // Requiring it at the envelope root would reject that valid shape before + // UIMCPServer flattens it; the shared gate enforces it after flattening. transactionId: z.number().int().optional().describe('Transaction ID to stop'), }), }, diff --git a/src/charging-station/ui-server/ui-services/AbstractUIService.ts b/src/charging-station/ui-server/ui-services/AbstractUIService.ts index 5c6c2a7f..f2b614e3 100644 --- a/src/charging-station/ui-server/ui-services/AbstractUIService.ts +++ b/src/charging-station/ui-server/ui-services/AbstractUIService.ts @@ -28,12 +28,14 @@ import { ensureError, getErrorMessage, isEmpty, + isJsonObject, isNotEmptyArray, JSONStringify, logger, } from '../../../utils/index.js' import { UIServiceWorkerBroadcastChannel } from '../../broadcast-channel/UIServiceWorkerBroadcastChannel.js' import { DEFAULT_MAX_STATIONS, isValidNumberOfStations } from '../UIServerSecurity.js' +import { getRequestPayloadValidationError } from './UIServiceRequestPayloadSchemas.js' const moduleName = 'AbstractUIService' @@ -234,6 +236,13 @@ export abstract class AbstractUIService { ) } + // Transport schemas may validate a different envelope; enforce the shared + // flat-payload contract before dispatching to stations. + const validationError = getRequestPayloadValidationError(command, requestPayload) + if (validationError != null) { + throw new BaseError(`'${command}' request payload is invalid: ${validationError}`) + } + // Call the request handler to build the response payload const requestHandler = this.requestHandlers.get(command) if (requestHandler == null) { @@ -249,7 +258,7 @@ export abstract class AbstractUIService { errorMessage: getErrorMessage(error), errorStack: error instanceof Error ? error.stack : undefined, hashIds: requestPayload?.hashIds, - requestPayload, + requestPayload: redactRequestCredentials(requestPayload), responsePayload, status: ResponseStatus.FAILURE, } satisfies ResponsePayload @@ -343,17 +352,6 @@ export abstract class AbstractUIService { status: ResponseStatus.FAILURE, } satisfies ResponsePayload } - if ( - typeof template !== 'string' || - typeof numberOfStations !== 'number' || - !Number.isInteger(numberOfStations) || - numberOfStations <= 0 - ) { - return { - errorMessage: 'Invalid request payload', - status: ResponseStatus.FAILURE, - } satisfies ResponsePayload - } if (!isValidNumberOfStations(numberOfStations, DEFAULT_MAX_STATIONS)) { return { errorMessage: `Number of stations must be between 1 and ${String(DEFAULT_MAX_STATIONS)}`, @@ -575,3 +573,38 @@ export abstract class AbstractUIService { } } } + +/** + * Validation failures bypass worker-side credential redaction. Use diagnostic + * copies to preserve caller-owned payloads. + * @param requestPayload - Original, potentially invalid request payload. + * @returns Diagnostics without root or object-options supervision credentials. + */ +const redactRequestCredentials = ( + requestPayload: RequestPayload | undefined +): RequestPayload | undefined => { + if (!isJsonObject(requestPayload)) { + return requestPayload + } + const options = requestPayload.options + const redactOptions = + isJsonObject(options) && + (Object.hasOwn(options, 'supervisionPassword') || Object.hasOwn(options, 'supervisionUser')) + if ( + !Object.hasOwn(requestPayload, 'supervisionPassword') && + !Object.hasOwn(requestPayload, 'supervisionUser') && + !redactOptions + ) { + return requestPayload + } + const redactedPayload = { ...requestPayload } + delete redactedPayload.supervisionPassword + delete redactedPayload.supervisionUser + if (redactOptions) { + const redactedOptions = { ...options } + delete redactedOptions.supervisionPassword + delete redactedOptions.supervisionUser + redactedPayload.options = redactedOptions + } + return redactedPayload +} diff --git a/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts b/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts new file mode 100644 index 00000000..067d093b --- /dev/null +++ b/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts @@ -0,0 +1,334 @@ +/** + * @file Canonical UI request payload schemas. + * @description Shared flat-payload gate for targeting and UI/worker control + * fields, enforced by `AbstractUIService.requestHandler` before dispatch. + * + * The OCPP layer owns version-specific PDU validation. Unlisted PDU fields + * pass through unchanged. MCP validates its envelope before flattening; + * each layer declares its own required fields while sharing field types. + */ + +import { z } from 'zod' + +import { ProcedureName } from '../../../types/index.js' +import { isEmpty } from '../../../utils/index.js' + +/** + * Omit to broadcast. An explicit empty array targets no station and is rejected + * by `AbstractUIService` rather than expanded into a broadcast. + */ +export const hashIdsField = z + .array(z.string()) + .optional() + .describe('Target station hash IDs (omit for all stations)') + +/** + * Allows station-level targets: OCPP 1.6 MeterValues uses 0 for the main power + * meter (§6.31), and StatusNotification uses 0 for the main controller (§6.47). + */ +const connectorIdField = z + .number() + .int() + .nonnegative() + .describe('OCPP connector ID (0 designates the charge point main controller/meter)') + +/** + * OCPP PDU EVSE identifier, OCPP 2.0.x counterpart of {@link connectorIdField}. + * + * Physical EVSE IDs start at 1. For station reporting, EVSE ID 0 is reserved + * for the main controller (OCPP 2.0.1 Part 1 §7.1). `MeterValuesRequest` + * specifically uses 0 for the main power meter (Part 2 §1.32.1). + * The shared UI field retains a non-negative bound; this does not establish + * which zero targets are valid for every OCPP message. + */ +const evseIdField = z + .number() + .int() + .nonnegative() + .describe('OCPP EVSE ID (0: reporting main controller; MeterValues main power meter)') + +/** + * OCPP 1.6 StartTransaction (§6.45) and UnlockConnector (§6.53) require a positive + * connector ID. Do not apply this bound to LockConnector: the simulator accepts + * its controller target 0 as a logged no-op. + */ +export const physicalConnectorIdField = z + .number() + .int() + .positive() + .describe('Connector ID required to be > 0 by OCPP 1.6 §6.45 / §6.53') + +/** Physical connector IDs, each subject to {@link physicalConnectorIdField}. */ +export const connectorIdsField = z + .array(physicalConnectorIdField) + .optional() + .describe('Target physical connector IDs') + +/** + * Validate URL syntax here; the supervision transport checks scheme support + * at connection time. Avoid throwing refinements so malformed URLs remain + * validation failures rather than exceptions. + */ +export const urlField = z.url() + +/** Supervision URL(s) of a charging station, single or array form. */ +const supervisionUrlsField = z.union([urlField, z.array(urlField)]) + +/** + * Match template semantics: `openWSConnection` accepts a colon-bearing user + * but omits authentication with a warning. The stricter `supervisionUserField` + * would reject configurations accepted by templates. + */ +const supervisionUserOptionField = z + .string() + .describe('CSMS basic auth user used on the supervision WebSocket') + +/** + * Basic-auth user typed explicitly by an operator through + * `SET_SUPERVISION_URL`. A colon would be ambiguous in the `user:password` + * credential (RFC 7617) and is refused here rather than silently downgraded to + * an unauthenticated connection. + */ +export const supervisionUserField = z + .string() + .regex(/^[^:]*$/, 'must not contain ":"') + .describe('CSMS basic auth user used on the supervision WebSocket') + +/** Basic-auth password for the supervision WebSocket. */ +export const supervisionPasswordField = z + .string() + .describe('CSMS basic auth password used on the supervision WebSocket') + +/** + * Station overrides. Unknown keys are accepted but omitted from parsed output: + * this gate retains the original payload, while the MCP SDK uses parsed output. + */ +export const chargingStationOptionsSchema = z.object({ + autoRegister: z.boolean().optional().describe('Set stations as registered at boot notification'), + autoStart: z.boolean().optional().describe('Enable automatic start of added charging station'), + baseName: z + .string() + .optional() + .describe('Override the template base name used to derive the charging station id'), + enableStatistics: z.boolean().optional().describe('Enable charging station statistics'), + fixedName: z + .boolean() + .optional() + .describe('Use base name verbatim as charging station id instead of appending index/suffix'), + nameSuffix: z + .string() + .optional() + .describe( + 'Suffix appended to the derived charging station id (ignored when fixed name is true)' + ), + ocppStrictCompliance: z + .boolean() + .optional() + .describe('Enable strict OCPP specifications adherence'), + persistentConfiguration: z + .boolean() + .optional() + .describe('Enable persistent OCPP parameters storage'), + stopTransactionsOnStopped: z + .boolean() + .optional() + .describe('Enable stop transactions on station stop'), + supervisionPassword: supervisionPasswordField.optional(), + supervisionUrls: supervisionUrlsField.optional().describe('OCPP server supervision URL(s)'), + supervisionUser: supervisionUserOptionField.optional(), +}) + +/** + * Only ATG procedures interpret `connectorIds`; other workers discard it. + * Declaring it here would reject otherwise ignored values. + */ +const broadcastFields = { hashIds: hashIdsField } as const + +/** Targeting fields of the two automatic transaction generator procedures. */ +const atgTargetFields = { connectorIds: connectorIdsField, hashIds: hashIdsField } as const + +/** + * For procedures without additional UI control fields, validate only station + * targeting and leave OCPP PDU fields to the protocol layer. + */ +const broadcastSchema = z.looseObject(broadcastFields) + +/** + * Canonical UI request payload schema per procedure. + * + * Typed as a total `Record` over `ProcedureName`: adding a procedure to the + * enum without declaring its payload shape is a compile-time error. + */ +const uiServiceRequestPayloadSchemas: Readonly> = { + [ProcedureName.ADD_CHARGING_STATIONS]: z.looseObject({ + numberOfStations: z.number().int().positive(), + options: chargingStationOptionsSchema.optional(), + template: z.string(), + }), + [ProcedureName.AUTHORIZE]: broadcastSchema, + [ProcedureName.BOOT_NOTIFICATION]: broadcastSchema, + [ProcedureName.CHANGE_CONFIGURATION]: z.looseObject({ + ...broadcastFields, + key: z.string().min(1), + value: z.string(), + }), + [ProcedureName.CLOSE_CONNECTION]: broadcastSchema, + [ProcedureName.DATA_TRANSFER]: broadcastSchema, + [ProcedureName.DELETE_CHARGING_STATIONS]: z.looseObject({ + ...broadcastFields, + deleteConfiguration: z.boolean().optional(), + }), + [ProcedureName.DIAGNOSTICS_STATUS_NOTIFICATION]: broadcastSchema, + [ProcedureName.FIRMWARE_STATUS_NOTIFICATION]: broadcastSchema, + [ProcedureName.GET_15118_EV_CERTIFICATE]: broadcastSchema, + [ProcedureName.GET_CERTIFICATE_STATUS]: broadcastSchema, + [ProcedureName.HEARTBEAT]: broadcastSchema, + [ProcedureName.LIST_CHARGING_STATIONS]: z.looseObject({}), + [ProcedureName.LIST_TEMPLATES]: z.looseObject({}), + [ProcedureName.LOCK_CONNECTOR]: z.looseObject({ + ...broadcastFields, + connectorId: connectorIdField, + }), + [ProcedureName.LOG_STATUS_NOTIFICATION]: broadcastSchema, + // Supplied meter values require sampled-value arrays; contents stay OCPP-owned. + // The target is a union, not an intersection: OCPP 1.6 MeterValues requires + // `connectorId` while OCPP 2.0.1 requires `evseId`, and the two enumerations + // share no mandatory member, so no single field can be required here. The + // rule below is the same condition `handleMeterValues` applies per station + // (`ChargingStationWorkerBroadcastChannel` throws `Missing connectorId or + // evseId`), so it rejects nothing the worker would have accepted: a payload + // with neither now fails once at the gate instead of once per station. + [ProcedureName.METER_VALUES]: z + .looseObject({ + ...broadcastFields, + connectorId: connectorIdField.optional(), + evseId: evseIdField.optional(), + meterValue: z.array(z.looseObject({ sampledValue: z.array(z.unknown()) })).optional(), + }) + .refine(payload => payload.connectorId != null || payload.evseId != null, { + message: 'at least one of "connectorId" or "evseId" is required', + path: ['connectorId'], + }), + [ProcedureName.NOTIFY_CUSTOMER_INFORMATION]: broadcastSchema, + [ProcedureName.NOTIFY_REPORT]: broadcastSchema, + [ProcedureName.OPEN_CONNECTION]: broadcastSchema, + [ProcedureName.PERFORMANCE_STATISTICS]: z.looseObject({}), + [ProcedureName.SECURITY_EVENT_NOTIFICATION]: broadcastSchema, + [ProcedureName.SET_SUPERVISION_URL]: z.looseObject({ + ...broadcastFields, + supervisionPassword: supervisionPasswordField.optional(), + supervisionUser: supervisionUserField.optional(), + url: urlField, + }), + [ProcedureName.SIGN_CERTIFICATE]: broadcastSchema, + [ProcedureName.SIMULATOR_STATE]: z.looseObject({}), + [ProcedureName.START_AUTOMATIC_TRANSACTION_GENERATOR]: z.looseObject(atgTargetFields), + [ProcedureName.START_CHARGING_STATION]: broadcastSchema, + [ProcedureName.START_SIMULATOR]: z.looseObject({}), + [ProcedureName.START_TRANSACTION]: z.looseObject({ + ...broadcastFields, + connectorId: physicalConnectorIdField, + idTag: z.string().optional(), + }), + // connectorId is shared by both versions; evseId belongs to OCPP 2.0.x and + // remains optional at this version-blind gate. + [ProcedureName.STATUS_NOTIFICATION]: z + .looseObject({ + ...broadcastFields, + connectorId: connectorIdField, + connectorStatus: z.string().optional(), + // OCPP 1.6 §6.47 requires `errorCode`, OCPP 2.0.1 + // `StatusNotificationRequest.json` does not: the gate is version-blind, + // so it must stay optional or a valid 2.0.x client would be rejected. + errorCode: z.string().optional(), + evseId: evseIdField.optional(), + status: z.string().optional(), + }) + .refine(payload => payload.connectorStatus != null || payload.status != null, { + message: 'at least one of "connectorStatus" or "status" is required', + path: [], + }), + [ProcedureName.STOP_AUTOMATIC_TRANSACTION_GENERATOR]: z.looseObject(atgTargetFields), + [ProcedureName.STOP_CHARGING_STATION]: broadcastSchema, + [ProcedureName.STOP_SIMULATOR]: z.looseObject({}), + // The 1.6-only worker resolves the connector from transactionId, so connector + // and EVSE identifiers are neither required nor interpreted here. + [ProcedureName.STOP_TRANSACTION]: z.looseObject({ + ...broadcastFields, + transactionId: z.number().int(), + }), + [ProcedureName.TRANSACTION_EVENT]: broadcastSchema, + [ProcedureName.UNLOCK_CONNECTOR]: z.looseObject({ + ...broadcastFields, + connectorId: physicalConnectorIdField, + }), +} + +/** + * Renders a Zod issue path as a log-safe dotted path, dropping array indices: + * `['hashIds', 3]` renders as `hashIds`, a path made of indices only as `[]`. + * @param issuePath - Zod issue path segments. + * @returns `` for an empty path, otherwise the normalized dotted path. + */ +const renderIssuePath = (issuePath: readonly PropertyKey[]): string => { + if (isEmpty(issuePath)) { + return '' + } + const properties = issuePath.filter(segment => typeof segment !== 'number') + return isEmpty(properties) ? '[]' : properties.map(segment => segment.toString()).join('.') +} + +/** + * Renders a single `path -> message -> occurrence count` group. + * @param path - Normalized dotted path the issues belong to. + * @param messages - Distinct messages of the group, with their occurrence count. + * @returns `path: message` for a lone issue, `path: message (+N more issue(s))` + * for repeated occurrences of one message, `path: msg1 / msg2 (N issue(s))` for + * several distinct messages. + */ +const renderIssueGroup = (path: string, messages: ReadonlyMap): string => { + // Groups are nonempty because they are created for an issue. + const entries = [...messages] + const [firstMessage, firstCount] = entries[0] + if (messages.size === 1) { + return firstCount > 1 + ? `${path}: ${firstMessage} (+${(firstCount - 1).toString()} more issue(s))` + : `${path}: ${firstMessage}` + } + const occurrences = entries.reduce((sum, [, count]) => sum + count, 0) + return `${path}: ${entries.map(([message]) => message).join(' / ')} (${occurrences.toString()} issue(s))` +} + +/** + * Groups issues by full field path, without array indices, to bound report + * entries for large arrays while retaining distinct nested-field violations. + * @param issues - Zod validation issues. + * @returns Semicolon-separated groups (see {@link renderIssueGroup}), in + * first-seen field order. + */ +const formatIssues = (issues: readonly z.core.$ZodIssue[]): string => { + const groups = new Map>() + for (const issue of issues) { + const path = renderIssuePath(issue.path) + const messages = groups.get(path) ?? new Map() + messages.set(issue.message, (messages.get(issue.message) ?? 0) + 1) + groups.set(path, messages) + } + return [...groups].map(([path, messages]) => renderIssueGroup(path, messages)).join('; ') +} + +/** + * Validates the procedure payload without transforming it: handlers rely on + * optional-field presence and unlisted PDU fields remaining unchanged. + * @param procedureName - Procedure the payload is destined for. + * @param requestPayload - Untrusted payload, as received from the transport. + * @returns `undefined` when the payload is valid, otherwise a single-line + * description of every violation. + */ +export const getRequestPayloadValidationError = ( + procedureName: ProcedureName, + requestPayload: unknown +): string | undefined => { + const result = uiServiceRequestPayloadSchemas[procedureName].safeParse(requestPayload) + return result.success ? undefined : formatIssues(result.error.issues) +} diff --git a/src/types/index.ts b/src/types/index.ts index 62576476..1f940391 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -109,6 +109,7 @@ export { type OCPP16ReserveNowRequest, type OCPP16SendLocalListRequest, type OCPP16StatusNotificationRequest, + type OCPP16StatusNotificationRequestParams, type OCPP16TriggerMessageRequest, type OCPP16UpdateFirmwareRequest, OCPP16UpdateType, diff --git a/src/types/ocpp/1.6/Requests.ts b/src/types/ocpp/1.6/Requests.ts index 1b45679c..faf186a4 100644 --- a/src/types/ocpp/1.6/Requests.ts +++ b/src/types/ocpp/1.6/Requests.ts @@ -180,6 +180,16 @@ export interface OCPP16StatusNotificationRequest extends JsonObject { vendorId?: string } +/** + * Accepts both UI status spellings: `status` (1.6) and `connectorStatus` (2.0.x). + * Values remain untrusted because not every 2.0.x status has a 1.6 counterpart; + * the builder validates them before producing a PDU. + */ +export type OCPP16StatusNotificationRequestParams = Partial & + Pick & { + connectorStatus?: string + } + export interface OCPP16TriggerMessageRequest extends JsonObject { connectorId?: number requestedMessage: OCPP16MessageTrigger diff --git a/tests/charging-station/ocpp/1.6/OCPP16ServiceUtils.test.ts b/tests/charging-station/ocpp/1.6/OCPP16ServiceUtils.test.ts index c5bf529d..9a7a1487 100644 --- a/tests/charging-station/ocpp/1.6/OCPP16ServiceUtils.test.ts +++ b/tests/charging-station/ocpp/1.6/OCPP16ServiceUtils.test.ts @@ -614,15 +614,50 @@ await describe('OCPP16ServiceUtils — pure functions', async () => { assert.strictEqual(result.errorCode, ChargePointErrorCode.CONNECTOR_LOCK_FAILURE) }) - await it('should pass through undefined errorCode when not set in payload', () => { - const input = { + await it('should resolve the inter-version connectorStatus alias', () => { + // Version-blind UI requests may use connectorStatus even for 1.6 stations. + const result = OCPP16ServiceUtils.buildStatusNotificationRequest({ connectorId: 1, + connectorStatus: OCPP16ChargePointStatus.Available, + errorCode: ChargePointErrorCode.NO_ERROR, + }) + + assert.strictEqual(result.status, OCPP16ChargePointStatus.Available) + }) + + await it('should let connectorStatus take precedence over status', () => { + const result = OCPP16ServiceUtils.buildStatusNotificationRequest({ + connectorId: 1, + connectorStatus: OCPP16ChargePointStatus.Faulted, + errorCode: ChargePointErrorCode.NO_ERROR, status: OCPP16ChargePointStatus.Available, - } as unknown as OCPP16StatusNotificationRequest + }) - const result = OCPP16ServiceUtils.buildStatusNotificationRequest(input) + assert.strictEqual(result.status, OCPP16ChargePointStatus.Faulted) + }) + + await it('should refuse a 2.0.1-only connector status', () => { + // Occupied cannot be encoded by the 1.6 StatusNotification schema. + assert.throws( + () => + OCPP16ServiceUtils.buildStatusNotificationRequest({ + connectorId: 1, + connectorStatus: 'Occupied', + errorCode: ChargePointErrorCode.NO_ERROR, + }), + OCPPError + ) + }) - assert.strictEqual(result.errorCode, undefined) + await it('should refuse a missing connector status', () => { + assert.throws( + () => + OCPP16ServiceUtils.buildStatusNotificationRequest({ + connectorId: 1, + errorCode: ChargePointErrorCode.NO_ERROR, + }), + OCPPError + ) }) }) diff --git a/tests/charging-station/ui-server/UIServerTestUtils.ts b/tests/charging-station/ui-server/UIServerTestUtils.ts index df55abdf..b1122d20 100644 --- a/tests/charging-station/ui-server/UIServerTestUtils.ts +++ b/tests/charging-station/ui-server/UIServerTestUtils.ts @@ -21,7 +21,6 @@ import type { ProcedureName, ProtocolRequest, ProtocolResponse, - ProtocolVersion, RequestPayload, UIServerConfiguration, UUIDv4, @@ -34,10 +33,11 @@ import { ApplicationProtocolVersion, AuthenticationType, type OCPPVersion, + ProtocolVersion, ResponseStatus, } from '../../../src/types/index.js' import { MockWebSocket } from '../mocks/MockWebSocket.js' -import { TEST_UUID } from './UIServerTestConstants.js' +import { TEST_HASH_ID, TEST_HASH_ID_2, TEST_UUID } from './UIServerTestConstants.js' export const createMockBootstrap = (): IBootstrap => ({ addChargingStation: () => Promise.resolve(undefined), @@ -237,6 +237,28 @@ export const createMockUIServerConfigurationWithAuth = ( }) } +/** + * Registers stations so unintended fan-out is observable through the + * outstanding responder count. + * @param stationCount - Number of stations to register (1 or 2 supported). + * @returns Server and registered UI service. + */ +export const createServiceContext = ( + stationCount: 1 | 2 = 1 +): { readonly server: TestableUIWebSocketServer; readonly service: AbstractUIService } => { + const server = new TestableUIWebSocketServer(createMockUIServerConfiguration()) + server.testRegisterProtocolVersionUIService(ProtocolVersion['0.0.1']) + const hashIds = stationCount === 1 ? [TEST_HASH_ID] : [TEST_HASH_ID, TEST_HASH_ID_2] + for (const hashId of hashIds) { + server.setChargingStationData(hashId, createMockChargingStationData(hashId)) + } + const service = server.getUIService(ProtocolVersion['0.0.1']) + if (service == null) { + assert.fail('Expected UI service to be registered') + } + return { server, service } +} + export class MockServerResponse extends EventEmitter { public body?: string public bodyBuffer?: Buffer diff --git a/tests/charging-station/ui-server/ui-services/AbstractUIService.test.ts b/tests/charging-station/ui-server/ui-services/AbstractUIService.test.ts index bc848424..f328ea43 100644 --- a/tests/charging-station/ui-server/ui-services/AbstractUIService.test.ts +++ b/tests/charging-station/ui-server/ui-services/AbstractUIService.test.ts @@ -24,26 +24,12 @@ import { createMockChargingStationData, createMockUIServerConfiguration, createProtocolRequest, + createServiceContext, emitWorkerResponse, expectSingleLog, TestableUIWebSocketServer, } from '../UIServerTestUtils.js' -const createServiceContext = (): { - readonly server: TestableUIWebSocketServer - readonly service: AbstractUIService -} => { - const config = createMockUIServerConfiguration() - const server = new TestableUIWebSocketServer(config) - server.testRegisterProtocolVersionUIService(ProtocolVersion['0.0.1']) - server.setChargingStationData(TEST_HASH_ID, createMockChargingStationData(TEST_HASH_ID)) - const service = server.getUIService(ProtocolVersion['0.0.1']) - if (service == null) { - assert.fail('Expected UI service to be registered') - } - return { server, service } -} - const registerInternalStopRequest = async (server: TestableUIWebSocketServer): Promise => { await server.sendInternalRequest( server.buildProtocolRequest(TEST_UUID, ProcedureName.STOP_CHARGING_STATION, {}) diff --git a/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts b/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts new file mode 100644 index 00000000..65a1876f --- /dev/null +++ b/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts @@ -0,0 +1,973 @@ +/** + * @file Tests for UI service request payload validation + * @description Exercises the shared flat-payload gate through the service + * dispatch point used by HTTP, WebSocket and MCP after transport validation. + */ + +import assert from 'node:assert/strict' +import { afterEach, describe, it } from 'node:test' + +import type { AbstractUIService } from '../../../../src/charging-station/ui-server/ui-services/AbstractUIService.js' +import type { + ProcedureName as ProcedureNameType, + ProtocolResponse, + RequestPayload, +} from '../../../../src/types/index.js' + +import { ProcedureName, ResponseStatus } from '../../../../src/types/index.js' +import { standardCleanup } from '../../../helpers/TestLifecycleHelpers.js' +import { TEST_HASH_ID, TEST_UUID } from '../UIServerTestConstants.js' +import { createProtocolRequest, createServiceContext } from '../UIServerTestUtils.js' + +/** + * Dispatch an untrusted payload the `RequestPayload` type cannot express. + * @param service - UI service under test. + * @param procedureName - Target procedure. + * @param payload - Untrusted payload, bypassing `RequestPayload` typing. + * @returns Protocol response, or `undefined` when the request is deferred. + */ +const dispatchUntrustedPayload = async ( + service: AbstractUIService, + procedureName: ProcedureNameType, + payload: unknown +): Promise => + await service.requestHandler([TEST_UUID, procedureName, payload as RequestPayload]) + +/** + * Returns the rejection message, asserting a synchronous failure response. + * @param response - Protocol response returned by the dispatch. + * @returns The reported error message. + */ +const failureErrorMessage = (response: ProtocolResponse | undefined): string => { + assert.notStrictEqual(response, undefined, 'Expected a synchronous protocol response') + if (response == null) { + return assert.fail('Expected a synchronous protocol response') + } + const { errorMessage, status } = response[1] + assert.strictEqual(status, ResponseStatus.FAILURE) + assert.strictEqual(typeof errorMessage, 'string') + return typeof errorMessage === 'string' ? errorMessage : assert.fail('Expected an error message') +} + +/** + * Asserts a failure response describing the expected violation. + * @param response - Protocol response returned by the dispatch. + * @param expectedViolation - Pattern the reported violation must match. + */ +const assertRejectedWith = ( + response: ProtocolResponse | undefined, + expectedViolation: RegExp +): void => { + assert.match(failureErrorMessage(response), expectedViolation) +} + +await describe('UIServiceRequestPayloadSchemas', async () => { + afterEach(() => { + standardCleanup() + }) + + await it('should not broadcast to every station when hashIds is not an array', async () => { + const { service } = createServiceContext(2) + + try { + // A targeted request whose selector has the wrong type must fail, never + // silently degrade into a broadcast over both registered stations. + const response = await dispatchUntrustedPayload( + service, + ProcedureName.STOP_CHARGING_STATION, + { + hashIds: TEST_HASH_ID, + } + ) + + assertRejectedWith(response, /hashIds: Invalid input: expected array/) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should target only the listed station when hashIds is a valid array', async () => { + const { service } = createServiceContext(2) + + try { + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.STOP_CHARGING_STATION, { + hashIds: [TEST_HASH_ID], + }) + ) + + // A broadcast is deferred: no synchronous response, and exactly the + // explicitly targeted station is tracked as an expected responder. + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should reject a payload that is not an object', async () => { + const { service } = createServiceContext(2) + + try { + // The HTTP transport only JSON-parses the body, so a scalar or an array + // reaches the service without ever being an object. + for (const payload of [42, 'text', null, []]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.LIST_TEMPLATES, payload), + /Invalid input: expected object/ + ) + } + } finally { + service.stop() + } + }) + + await it('should reject a mistyped control field', async () => { + const { service } = createServiceContext(2) + + try { + const response = await dispatchUntrustedPayload(service, ProcedureName.LOCK_CONNECTOR, { + connectorId: 'one', + hashIds: [TEST_HASH_ID], + }) + + assertRejectedWith(response, /connectorId: Invalid input: expected number/) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should reject a missing required control field', async () => { + const { service } = createServiceContext(2) + + try { + const response = await dispatchUntrustedPayload(service, ProcedureName.CHANGE_CONFIGURATION, { + hashIds: [TEST_HASH_ID], + value: '120', + }) + + assertRejectedWith(response, /key: Invalid input: expected string/) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should reject a non-boolean deleteConfiguration instead of coercing it', async () => { + const { service } = createServiceContext(2) + + try { + // A truthy non-boolean must not authorize deleting persisted configuration. + const response = await dispatchUntrustedPayload( + service, + ProcedureName.DELETE_CHARGING_STATIONS, + { + deleteConfiguration: 'no', + hashIds: [TEST_HASH_ID], + } + ) + + assertRejectedWith(response, /deleteConfiguration: Invalid input: expected boolean/) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should forward unlisted OCPP PDU fields untouched', async () => { + const { service } = createServiceContext(2) + + try { + // The OCPP layer owns PDU validation against the OCPP JSON schemas. The UI + // payload gate must not reject, strip or rewrite those fields. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.START_TRANSACTION, { + connectorId: 1, + hashIds: [TEST_HASH_ID], + idTag: 'id-tag', + meterStart: 1_700_000_000_000, + ocppSequenceNumber: 7, + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should require a connector target for startTransaction', async () => { + const { service } = createServiceContext(2) + + try { + // `handleStartTransaction` requires connectorId; the gate must reject it + // once instead of letting a per-station failure surface the omission. + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.START_TRANSACTION, { + hashIds: [TEST_HASH_ID], + }), + /connectorId/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should require a connector target for statusNotification', async () => { + const { service } = createServiceContext(2) + + try { + // `handleStatusNotification` requires connectorId for the same reason. + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.STATUS_NOTIFICATION, { + hashIds: [TEST_HASH_ID], + status: 'Available', + }), + /connectorId/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should reject a non-URL supervision url like the MCP tool contract does', async () => { + const { service } = createServiceContext(2) + + try { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.SET_SUPERVISION_URL, { + hashIds: [TEST_HASH_ID], + url: 'not-a-url', + }), + /url/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should accept a websocket supervision url', async () => { + const { service } = createServiceContext(2) + + try { + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.SET_SUPERVISION_URL, { + hashIds: [TEST_HASH_ID], + url: 'ws://localhost:9999/OCPP16', + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should require a transactionId for stopTransaction', async () => { + const { service } = createServiceContext(2) + + try { + // `handleStopTransaction` resolves the connector from the transaction, so + // transactionId is the required field, not connectorId. + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.STOP_TRANSACTION, { + hashIds: [TEST_HASH_ID], + }), + /transactionId/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should keep reporting an unknown procedure as unimplemented', async () => { + const { service } = createServiceContext(2) + + try { + // Validation must not shadow the "not implemented" branch: an unknown + // procedure has no schema to check against. + const response = await dispatchUntrustedPayload( + service, + 'UnknownProcedure' as ProcedureNameType, + { hashIds: 'not-an-array' } + ) + + assertRejectedWith(response, /is not implemented/) + } finally { + service.stop() + } + }) + await it('should reject connector 0 where OCPP 1.6 mandates a strictly positive connector', async () => { + const { service } = createServiceContext(2) + + try { + // OCPP 1.6 §6.45 and §6.53 require physical connector IDs (> 0). + for (const procedureName of [ + ProcedureName.START_TRANSACTION, + ProcedureName.UNLOCK_CONNECTOR, + ]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, procedureName, { + connectorId: 0, + hashIds: [TEST_HASH_ID], + }), + /connectorId/ + ) + } + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should accept connector 0 for lockConnector, absent any specification bound', async () => { + const { service } = createServiceContext(2) + + try { + // The worker treats controller target 0 as a logged no-op; the flat gate + // must preserve that accepted target. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.LOCK_CONNECTOR, { + connectorId: 0, + hashIds: [TEST_HASH_ID], + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should bound connectorIds only where the worker reads it', async () => { + const { service } = createServiceContext(2) + + try { + // Workers discard connectorIds outside ATG, so only ATG validates it. + assertRejectedWith( + await dispatchUntrustedPayload( + service, + ProcedureName.START_AUTOMATIC_TRANSACTION_GENERATOR, + { + connectorIds: [0], + } + ), + /connectorIds/ + ) + + for (const procedureName of [ProcedureName.BOOT_NOTIFICATION, ProcedureName.HEARTBEAT]) { + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, procedureName, { + connectorIds: [0], + hashIds: [TEST_HASH_ID], + }) + ) + + assert.strictEqual(response, undefined) + } + } finally { + service.stop() + } + }) + + await it('should accept a colon-bearing supervision user in a station option', async () => { + const { service } = createServiceContext(2) + + try { + // Template-compatible options must not inherit the stricter URL-edit rule. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.ADD_CHARGING_STATIONS, { + numberOfStations: 1, + options: { supervisionUser: 'dom:admin' }, + template: 'test.station-template', + }) + ) + + assert.notStrictEqual(response, undefined) + if (response == null) return + const [, responsePayload] = response + const { errorMessage } = responsePayload + if (typeof errorMessage !== 'string') { + assert.fail('Expected a string errorMessage') + } + assert.doesNotMatch( + errorMessage, + /supervisionUser/, + 'the colon rule must not apply to a station option' + ) + } finally { + service.stop() + } + }) + + await it('should accept the main controller pseudo-connector as a PDU connector', async () => { + const { service } = createServiceContext(2) + + try { + // OCPP 1.6 §6.47 reserves connector 0 for the charge point main controller. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.STATUS_NOTIFICATION, { + connectorId: 0, + hashIds: [TEST_HASH_ID], + status: 'Available', + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should reject a connector id that is neither positive nor integral', async () => { + const { service } = createServiceContext(2) + + try { + for (const connectorId of [-1, 1.5]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.STATUS_NOTIFICATION, { + connectorId, + hashIds: [TEST_HASH_ID], + status: 'Available', + }), + /connectorId/ + ) + } + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should reject an empty configuration key', async () => { + const { service } = createServiceContext(2) + + try { + // An empty key names no configuration item; it must not be forwarded as + // a valid target. + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.CHANGE_CONFIGURATION, { + hashIds: [TEST_HASH_ID], + key: '', + value: '120', + }), + /key/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should require an integer transactionId for stopTransaction', async () => { + const { service } = createServiceContext(2) + + try { + // OCPP 1.6 StopTransaction.req types transactionId as an integer, and + // `handleStopTransaction` only dispatches to 1.6 stations. + for (const transactionId of ['42', 4.2]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.STOP_TRANSACTION, { + hashIds: [TEST_HASH_ID], + transactionId, + }), + /transactionId/ + ) + } + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should reject a mistyped or out-of-range addChargingStations payload', async () => { + const { service } = createServiceContext(2) + + try { + for (const payload of [ + { numberOfStations: 1, template: 42 }, + { numberOfStations: 0, template: 'template' }, + { numberOfStations: 1.5, template: 'template' }, + ]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.ADD_CHARGING_STATIONS, payload), + /numberOfStations|template/ + ) + } + } finally { + service.stop() + } + }) + + await it('should reject meter value containers that are not arrays', async () => { + const { service } = createServiceContext(2) + + try { + for (const meterValue of [{ sampledValue: [{ value: '1' }] }, { sampledValue: '1' }]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.METER_VALUES, { + connectorId: 1, + hashIds: [TEST_HASH_ID], + meterValue, + }), + /meterValue/ + ) + } + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should reject a meterValues entry without sampledValue before broadcasting', async () => { + const { service } = createServiceContext(2) + + try { + for (const payload of [ + { connectorId: 1, meterValue: [{}] }, + { + evseId: 1, + hashIds: [TEST_HASH_ID], + meterValue: [{ sampledValue: [{ value: '1' }] }, {}], + }, + ]) { + const response = await dispatchUntrustedPayload( + service, + ProcedureName.METER_VALUES, + payload + ) + + assertRejectedWith(response, /meterValue.*sampledValue/) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } + } finally { + service.stop() + } + }) + + await it('should dispatch a request for current values when meterValue is omitted', async () => { + const { service } = createServiceContext(2) + + try { + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.METER_VALUES, { + connectorId: 1, + hashIds: [TEST_HASH_ID], + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should accept evseId zero on meterValues', async () => { + const { service } = createServiceContext(2) + + try { + // OCPP 2.0.1 MeterValuesRequest: evseId 0 designates the main power meter. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.METER_VALUES, { + evseId: 0, + hashIds: [TEST_HASH_ID], + meterValue: [{ sampledValue: [{ value: '1' }] }], + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should require a connector or EVSE target on meterValues', async () => { + const { service } = createServiceContext(2) + + try { + // MeterValues requires connectorId in 1.6 or evseId in 2.0.1; this + // version-blind gate requires at least one. + for (const payload of [{ hashIds: [TEST_HASH_ID] }, {}]) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.METER_VALUES, payload), + /at least one of "connectorId" or "evseId" is required/ + ) + } + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should accept each meterValues target form alone and target one station', async () => { + const { service } = createServiceContext(2) + + try { + for (const payload of [{ connectorId: 0 }, { connectorId: 1 }, { evseId: 1 }]) { + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.METER_VALUES, { + ...payload, + hashIds: [TEST_HASH_ID], + meterValue: [{ sampledValue: [{ value: '1' }] }], + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } + } finally { + service.stop() + } + }) + + await it('should accept a startTransaction payload without idTag', async () => { + const { service } = createServiceContext(2) + + try { + // `OCPP16RequestService` fills `idTag` with the default `00000000`, so the + // produced PDU is valid without it. Requiring it here would additionally + // break the shipped Web UI, which sends `{ connectorId }` alone. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.START_TRANSACTION, { + connectorId: 1, + hashIds: [TEST_HASH_ID], + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should require a connector status on statusNotification', async () => { + const { service } = createServiceContext(2) + + try { + // Neither the 1.6 status nor the 2.0.x connectorStatus is supplied. + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.STATUS_NOTIFICATION, { + connectorId: 1, + hashIds: [TEST_HASH_ID], + }), + /at least one of "connectorStatus" or "status" is required/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should accept statusNotification without errorCode and reject a mistyped one', async () => { + const { service } = createServiceContext(2) + + try { + // `errorCode` is mandatory in OCPP 1.6 §6.47 but absent from the OCPP + // 2.0.1 request, and the gate is version-blind, so it stays optional. + for (const payload of [ + { connectorId: 1, hashIds: [TEST_HASH_ID], status: 'Available' }, + { connectorId: 1, errorCode: 'NoError', hashIds: [TEST_HASH_ID], status: 'Available' }, + ]) { + const response = await dispatchUntrustedPayload( + service, + ProcedureName.STATUS_NOTIFICATION, + payload + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.STATUS_NOTIFICATION, { + connectorId: 1, + errorCode: 42, + hashIds: [TEST_HASH_ID], + status: 'Available', + }), + /errorCode/ + ) + } finally { + service.stop() + } + }) + + await it('should ignore a foreign evseId on stopTransaction instead of rejecting it', async () => { + const { service } = createServiceContext(2) + + try { + // The worker resolves the connector from the transaction and never reads + // an EVSE identifier, so the field is neither required nor interpreted. + const response = await service.requestHandler( + createProtocolRequest(TEST_UUID, ProcedureName.STOP_TRANSACTION, { + evseId: 'foo', + hashIds: [TEST_HASH_ID], + transactionId: 1, + }) + ) + + assert.strictEqual(response, undefined) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 1) + } finally { + service.stop() + } + }) + + await it('should reject a supervision user containing a colon', async () => { + const { service } = createServiceContext(2) + + try { + // A colon would make the `user:password` credential of RFC 7617 ambiguous. + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.SET_SUPERVISION_URL, { + hashIds: [TEST_HASH_ID], + supervisionUser: 'a:b', + url: 'ws://localhost:1/', + }), + /supervisionUser: must not contain ":"/ + ) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should redact credentials from a rejected frozen supervision update without mutating it', async () => { + // Arrange + const { server, service } = createServiceContext() + const hashIds = Object.freeze([TEST_HASH_ID]) + const diagnostic = Object.freeze({ source: 'operator' }) + const payload = Object.freeze({ + diagnostic, + hashIds, + supervisionPassword: 'synthetic-password-top', + supervisionUser: 'synthetic:user-top', + url: 'ws://localhost:1/', + }) + + try { + // Act + const response = await dispatchUntrustedPayload( + service, + ProcedureName.SET_SUPERVISION_URL, + payload + ) + + // Assert + assertRejectedWith(response, /supervisionUser:/) + assert.ok(response) + assert.strictEqual(response[0], TEST_UUID) + assert.deepStrictEqual(response[1].requestPayload, { diagnostic, hashIds, url: payload.url }) + assert.deepStrictEqual(payload, { + diagnostic: { source: 'operator' }, + hashIds: [TEST_HASH_ID], + supervisionPassword: 'synthetic-password-top', + supervisionUser: 'synthetic:user-top', + url: 'ws://localhost:1/', + }) + const serialized = JSON.stringify(response) + assert.ok(!serialized.includes(payload.supervisionUser)) + assert.ok(!serialized.includes(payload.supervisionPassword)) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + server.stop() + } + }) + + await it('should redact rejected station option credentials while preserving the original options', async () => { + const { server, service } = createServiceContext() + const payload = { + numberOfStations: 0, + options: { + autoStart: true, + diagnostic: { source: 'operator' }, + supervisionPassword: 'synthetic-password-options', + supervisionUser: 'synthetic-user-options', + }, + template: 'test.station-template', + } + + try { + const response = await dispatchUntrustedPayload( + service, + ProcedureName.ADD_CHARGING_STATIONS, + payload + ) + + assertRejectedWith(response, /numberOfStations:/) + assert.ok(response) + assert.strictEqual(response[0], TEST_UUID) + assert.deepStrictEqual(response[1].requestPayload, { + numberOfStations: 0, + options: { autoStart: true, diagnostic: { source: 'operator' } }, + template: 'test.station-template', + }) + assert.deepStrictEqual(payload, { + numberOfStations: 0, + options: { + autoStart: true, + diagnostic: { source: 'operator' }, + supervisionPassword: 'synthetic-password-options', + supervisionUser: 'synthetic-user-options', + }, + template: 'test.station-template', + }) + const serialized = JSON.stringify(response) + assert.ok(!serialized.includes('synthetic-user-options')) + assert.ok(!serialized.includes('synthetic-password-options')) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + server.stop() + } + }) + + await it('should preserve invalid option shapes while redacting malformed top-level credentials', async () => { + for (const options of [null, 'invalid-options', ['invalid-options']]) { + // Arrange + const { server, service } = createServiceContext() + const supervisionPassword = Object.freeze({ value: 'synthetic-password-object' }) + const supervisionUser = Object.freeze(['synthetic-user-array']) + const payload = Object.freeze({ + numberOfStations: 0, + options, + supervisionPassword, + supervisionUser, + }) + + try { + // Act + const response = await dispatchUntrustedPayload( + service, + ProcedureName.ADD_CHARGING_STATIONS, + payload + ) + + // Assert + assertRejectedWith(response, /options:/) + assert.ok(response) + assert.deepStrictEqual(response[1].requestPayload, { numberOfStations: 0, options }) + assert.deepStrictEqual(payload, { + numberOfStations: 0, + options, + supervisionPassword: { value: 'synthetic-password-object' }, + supervisionUser: ['synthetic-user-array'], + }) + const serialized = JSON.stringify(response) + assert.ok(!serialized.includes('synthetic-password-object')) + assert.ok(!serialized.includes('synthetic-user-array')) + } finally { + server.stop() + } + } + }) + + await it('should name every violated sub-field of a nested object', async () => { + const { service } = createServiceContext(2) + + try { + // Distinct nested fields must not collapse into a single options entry. + const response = await dispatchUntrustedPayload( + service, + ProcedureName.ADD_CHARGING_STATIONS, + { + numberOfStations: 1, + options: { + autoRegister: 'yes', + autoStart: 'yes', + baseName: 1, + enableStatistics: 'yes', + fixedName: 'yes', + nameSuffix: 1, + ocppStrictCompliance: 'yes', + persistentConfiguration: 'yes', + stopTransactionsOnStopped: 'yes', + supervisionPassword: 1, + supervisionUrls: 1, + supervisionUser: 1, + }, + template: 42, + } + ) + const message = failureErrorMessage(response) + + assert.match(message, /template: /) + for (const subField of [ + 'autoRegister', + 'autoStart', + 'baseName', + 'enableStatistics', + 'fixedName', + 'nameSuffix', + 'ocppStrictCompliance', + 'persistentConfiguration', + 'stopTransactionsOnStopped', + 'supervisionPassword', + 'supervisionUrls', + 'supervisionUser', + ]) { + assert.match( + message, + new RegExp(`options\\.${subField}: `), + `Missing an entry naming options.${subField}` + ) + } + // One entry per violated field, so the report stays bounded by the + // declared fields rather than by the number of issues. + assert.strictEqual(message.split('; ').length, 13) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should render one entry per violated field path', async () => { + const { service } = createServiceContext(2) + + try { + const message = failureErrorMessage( + await dispatchUntrustedPayload(service, ProcedureName.CHANGE_CONFIGURATION, { + hashIds: [TEST_HASH_ID], + key: '', + value: 42, + }) + ) + + assert.match(message, /key: /) + assert.match(message, /value: /) + assert.strictEqual(message.split('; ').length, 2) + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + + await it('should bound the report when a station targeting array is entirely invalid', async () => { + const { service } = createServiceContext(2) + + try { + // Large invalid arrays must not produce one report entry per element. + const message = failureErrorMessage( + await dispatchUntrustedPayload(service, ProcedureName.HEARTBEAT, { + hashIds: new Array(20000).fill(42), + }) + ) + + assert.match(message, /hashIds: /) + assert.match(message, /\+19999 more issue\(s\)/) + } finally { + service.stop() + } + }) +}) diff --git a/ui/web/src/shared/composables/useSetUrlForm.ts b/ui/web/src/shared/composables/useSetUrlForm.ts index d8e4ac05..2e9d43ed 100644 --- a/ui/web/src/shared/composables/useSetUrlForm.ts +++ b/ui/web/src/shared/composables/useSetUrlForm.ts @@ -1,8 +1,16 @@ -import { readonly, ref, type Ref } from 'vue' +import { type MaybeRefOrGetter, readonly, ref, type Ref, toValue } from 'vue' import { useToast } from 'vue-toast-notification' import { useUIClient } from '@/core/index.js' +/** + * Stored credentials used to omit unchanged fields from URL updates. + */ +export interface SetUrlFormBaseCredentials { + supervisionPassword?: string + supervisionUser?: string +} + export interface SetUrlFormState { supervisionPassword: string supervisionUrl: string @@ -13,11 +21,14 @@ export interface SetUrlFormState { * Returns form state and submission logic for setting the supervision URL. * @param hashId - The charging station hash identifier * @param chargingStationId - The charging station display identifier + * @param baseCredentials - Stored credentials. Without a base, empty fields + * explicitly clear the stored values. * @returns Form state and submit/reset functions */ export function useSetUrlForm ( hashId: string, - chargingStationId: string + chargingStationId: string, + baseCredentials?: MaybeRefOrGetter ): { chargingStationId: string formState: Ref @@ -36,6 +47,21 @@ export function useSetUrlForm ( formState.value = makeInitialState() } + /** + * Omit unchanged credentials to avoid rewriting inherited values. Without a + * base value, an empty field explicitly clears the stored credential. + * @param field - Credential field being submitted. + * @param value - Value currently held by the form. + * @returns The value to send, `undefined` when it equals the station base. + */ + function credentialToSubmit ( + field: keyof SetUrlFormBaseCredentials, + value: string + ): string | undefined { + const base = toValue(baseCredentials)?.[field] + return base != null && base === value ? undefined : value + } + /** * Validates and submits the supervision URL update. * @returns Whether the submission was successful @@ -46,13 +72,18 @@ export function useSetUrlForm ( $toast.error('Supervision url is required') return false } + const supervisionUser = credentialToSubmit('supervisionUser', formState.value.supervisionUser) + if (supervisionUser?.includes(':') === true) { + $toast.error('Supervision username must not contain ":"') + return false + } pending.value = true try { await $uiClient.setSupervisionUrl( hashId, formState.value.supervisionUrl, - formState.value.supervisionUser, - formState.value.supervisionPassword + supervisionUser, + credentialToSubmit('supervisionPassword', formState.value.supervisionPassword) ) $toast.success('Supervision url successfully set') return true diff --git a/ui/web/src/skins/modern/components/dialogs/SetSupervisionUrlDialog.vue b/ui/web/src/skins/modern/components/dialogs/SetSupervisionUrlDialog.vue index 4c0252d9..a19dc847 100644 --- a/ui/web/src/skins/modern/components/dialogs/SetSupervisionUrlDialog.vue +++ b/ui/web/src/skins/modern/components/dialogs/SetSupervisionUrlDialog.vue @@ -47,7 +47,8 @@ placeholder="Password" > - Credentials are sent verbatim; leaving username or password empty clears the stored value. + Unchanged credentials are kept as they are. Clear a field to remove the stored value. The + username must not contain ":" (RFC 7617).