From 4224a9349e1345f4d4a3348eab064c6970db96e4 Mon Sep 17 00:00:00 2001 From: Vitor Coelho <6161176+VitorCoelho@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:39:54 +0000 Subject: [PATCH] fix(simulator)!: reject supervision URLs whose scheme the ws client cannot dial (#2160) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Refactor supervisionUrls to use SupervisionUrlSchema Updated supervisionUrls to use SupervisionUrlSchema for validation. * Add SupervisionUrlSchema to index export * fix(ui-server): validate the supervision URL scheme at the UI request gate Signed-off-by: VitorCoelho * Add test for rejecting invalid supervision URLs * test(utils): assert the supervision URL schemes match the ws client Signed-off-by: VitorCoelho * docs(readme): document the accepted supervisionUrls schemes Signed-off-by: VitorCoelho * fix(simulator)!: bump the configuration schema version to 2 v2 narrows `supervisionUrls` to URLs whose scheme the ws client can dial. The v1 to v2 migration is a pass-through: values outside that set cannot be rewritten automatically and are reported by schema validation. BREAKING CHANGE: a configuration file whose `supervisionUrls` uses a scheme the ws client cannot dial now fails validation at startup instead of failing on each connection attempt. Signed-off-by: VitorCoelho * chore(config): stamp the configuration template with schema version 2 Signed-off-by: VitorCoelho * test(utils): cover the v1 to v2 configuration migration Signed-off-by: VitorCoelho * test(utils): cover v1 configurations under schema version 2 Signed-off-by: VitorCoelho * docs(readme): document configuration schema version 2 Signed-off-by: VitorCoelho * fix(simulator): reject supervision URLs with a fragment or an empty ws+unix socket path The ws client throws for a URL with a fragment identifier and for a ws+unix URL without a pathname, so these values passed the gate and failed per station at connection time. Reject them in SupervisionUrlSchema. Signed-off-by: VitorCoelho * test(utils): cover fragments and ws+unix socket paths in the supervision URL schema Add a fragment URL and ws+unix URLs without a socket path to the ws parity test, together with a ws+unix URL with a request path and a hostless network URL, which the ws client dials. Signed-off-by: VitorCoelho --------- Signed-off-by: VitorCoelho Co-authored-by: Jérôme Benoit --- README.md | 4 +- src/assets/config-template.json | 2 +- .../UIServiceRequestPayloadSchemas.ts | 10 +- src/utils/ConfigurationMigrations.ts | 15 ++- src/utils/ConfigurationSchema.ts | 32 ++++++- src/utils/index.ts | 1 + .../UIServiceRequestPayloadSchemas.test.ts | 19 ++++ tests/utils/ConfigurationMigrations.test.ts | 13 ++- tests/utils/ConfigurationSchema.test.ts | 95 +++++++++++++++++++ tests/utils/ConfigurationValidation.test.ts | 38 +++++++- 10 files changed, 214 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 99182f47..376acfc6 100644 --- a/README.md +++ b/README.md @@ -175,8 +175,8 @@ But the modifications to test have to be done to the files in the build target d | Key | Value(s) | Default Value | Value type | Description | | -------------------------- | -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| $schemaVersion | 1 | 1 | integer | Configuration schema version. Set to 1. Files without this field are migrated from v0; deprecated keys are remapped on every load. | -| supervisionUrls | | [] | string \| string[] | string or strings array containing global connection URIs to OCPP-J servers | +| $schemaVersion | 2 | 2 | integer | Configuration schema version. Set to 2. Files without this field are migrated from v0; deprecated keys are remapped on every load. | +| supervisionUrls | | [] | string \| string[] | string or strings array containing global connection URIs to OCPP-J servers (`ws://`, `wss://`, `http://`, `https://` or `ws+unix://`) | | supervisionUrlDistribution | round-robin/random/charging-station-affinity | charging-station-affinity | string | supervision urls distribution policy to simulated charging stations | | log | | {
"enabled": true,
"file": "logs/combined.log",
"errorFile": "logs/error.log",
"statisticsInterval": 60,
"level": "info",
"console": false,
"format": "simple",
"rotate": true
} | {
enabled?: boolean;
file?: string;
errorFile?: string;
statisticsInterval?: number;
level?: string;
console?: boolean;
format?: string;
rotate?: boolean;
maxFiles?: string \| number;
maxSize?: string \| number;
} | Log configuration section:
- _enabled_: enable logging
- _file_: log file relative path
- _errorFile_: error log file relative path
- _statisticsInterval_: seconds between charging stations statistics output in the logs
- _level_: emerg/alert/crit/error/warning/notice/info/debug [winston](https://github.com/winstonjs/winston) logging level
- _console_: output logs on the console
- _format_: [winston](https://github.com/winstonjs/winston) log format
- _rotate_: enable daily log files rotation
- _maxFiles_: maximum number of log files: https://github.com/winstonjs/winston-daily-rotate-file#options
- _maxSize_: maximum size of log files in bytes, or units of kb, mb, and gb: https://github.com/winstonjs/winston-daily-rotate-file#options | | worker | | {
"processType": "workerSet",
"startDelay": 500,
"elementAddDelay": 0,
"elementsPerWorker": 'auto',
"poolMinSize": 4,
"poolMaxSize": 16
} | {
processType?: WorkerProcessType;
startDelay?: number;
elementAddDelay?: number;
elementsPerWorker?: number \| 'auto' \| 'all';
poolMinSize?: number;
poolMaxSize?: number;
resourceLimits?: ResourceLimits;
} | Worker configuration section:
- _processType_: worker threads process type (`workerSet`/`fixedPool`/`dynamicPool`)
- _startDelay_: milliseconds to wait at worker threads startup (only for `workerSet` worker threads process type)
- _elementAddDelay_: milliseconds to wait between charging station add
- _elementsPerWorker_: number of charging stations per worker threads for the `workerSet` process type (`auto` means (number of stations) / (number of CPUs) \* 1.5 if (number of stations) > (number of CPUs), otherwise 1; `all` means a unique worker will run all charging stations)
- _poolMinSize_: worker threads pool minimum number of threads
- _poolMaxSize_: worker threads pool maximum number of threads
- _resourceLimits_: worker threads [resource limits](https://nodejs.org/api/worker_threads.html#new-workerfilename-options) object option | diff --git a/src/assets/config-template.json b/src/assets/config-template.json index 613e44eb..f077eca9 100644 --- a/src/assets/config-template.json +++ b/src/assets/config-template.json @@ -1,5 +1,5 @@ { - "$schemaVersion": 1, + "$schemaVersion": 2, "supervisionUrls": ["ws://localhost:8010/OCPP16/5be7fb271014d90008992f06"], "supervisionUrlDistribution": "round-robin", "log": { diff --git a/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts b/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts index 067d093b..2e30ea8a 100644 --- a/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts +++ b/src/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.ts @@ -11,7 +11,7 @@ import { z } from 'zod' import { ProcedureName } from '../../../types/index.js' -import { isEmpty } from '../../../utils/index.js' +import { isEmpty, SupervisionUrlSchema } from '../../../utils/index.js' /** * Omit to broadcast. An explicit empty array targets no station and is rejected @@ -65,11 +65,11 @@ export const connectorIdsField = z .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. + * Validate URL syntax and scheme here so a value the supervision transport + * cannot dial is rejected at the gate instead of failing per station at + * connection time. Shared with the configuration file `supervisionUrls`. */ -export const urlField = z.url() +export const urlField = SupervisionUrlSchema /** Supervision URL(s) of a charging station, single or array form. */ const supervisionUrlsField = z.union([urlField, z.array(urlField)]) diff --git a/src/utils/ConfigurationMigrations.ts b/src/utils/ConfigurationMigrations.ts index 548125fa..f2e36da4 100644 --- a/src/utils/ConfigurationMigrations.ts +++ b/src/utils/ConfigurationMigrations.ts @@ -54,7 +54,7 @@ export const DEPRECATED_KEY_REMAPPINGS: Readonly> * Bump only on breaking changes (field rename, removal, type narrowing). * Single authoritative location — concurrent bumps force git merge conflict. */ -export const CURRENT_CONFIGURATION_SCHEMA_VERSION = 1 +export const CURRENT_CONFIGURATION_SCHEMA_VERSION = 2 export type MigrationFn = ( config: Record, @@ -224,12 +224,23 @@ export const remapDeprecatedKeys = (config: Record): RemapDepre */ const migrateV0ToV1: MigrationFn = (config, _filePath) => clone(config) +/** + * v1 → v2: pure version-bump. v2 narrows `supervisionUrls` to URLs whose + * scheme the `ws` client can dial; values outside that set cannot be + * rewritten automatically and are reported by schema validation. + * `$schemaVersion` is stamped by `applyConfigurationMigration`. + * @param config - source configuration object + * @param _filePath - configuration file path (unused) + * @returns new configuration object + */ +const migrateV1ToV2: MigrationFn = (config, _filePath) => clone(config) + /** * Sequential migration chain. Index `i` migrates a v`i` configuration to v`i+1`. * To add schema version N+1: append one `migrateV{N}ToV{N+1}` function and * bump `CURRENT_CONFIGURATION_SCHEMA_VERSION`. */ -export const migrationChain: readonly MigrationFn[] = [migrateV0ToV1] +export const migrationChain: readonly MigrationFn[] = [migrateV0ToV1, migrateV1ToV2] /** * Strict integer-string pattern for `$schemaVersion`. Rejects permissive diff --git a/src/utils/ConfigurationSchema.ts b/src/utils/ConfigurationSchema.ts index 4a03d73d..2ec0f4b9 100644 --- a/src/utils/ConfigurationSchema.ts +++ b/src/utils/ConfigurationSchema.ts @@ -285,6 +285,36 @@ export const StationTemplateUrlSchema = z }) .strict() +/** + * Supervision URL — a URL that the `ws` WebSocket client can dial: scheme + * `ws:`, `wss:`, `http:`, `https:` or `ws+unix:`, no fragment identifier, and + * a socket path for `ws+unix:`. A scheme-less `host:port/path` string parses + * as a URL with the host as its scheme, so syntax validation alone would + * accept a value that fails at connection time. + * Shared by the configuration file and the UI request payload gate so a value + * is accepted by both or neither. + */ +export const SupervisionUrlSchema = z + .url({ + error: 'must be a WebSocket URL (ws://, wss://, http://, https:// or ws+unix://)', + protocol: /^(?:wss?|https?|ws\+unix)$/, + }) + .refine(value => !URL.canParse(value) || new URL(value).hash === '', { + message: 'must not contain a fragment identifier (#...)', + }) + .refine( + value => { + if (!URL.canParse(value)) { + return true + } + const url = new URL(value) + return url.protocol !== 'ws+unix:' || url.pathname !== '' + }, + { + message: 'ws+unix:// URLs must include the socket path', + } + ) + // --------------------------------------------------------------- // Top-level configuration schema // --------------------------------------------------------------- @@ -356,7 +386,7 @@ export const ConfigurationSchema = z .union([z.string(), z.array(z.string())]) .optional() .describe("@deprecated: use 'supervisionUrls' instead"), - supervisionUrls: z.union([z.string(), z.array(z.string())]).optional(), + supervisionUrls: z.union([SupervisionUrlSchema, z.array(SupervisionUrlSchema)]).optional(), uiServer: UIServerConfigurationSchema.optional(), uiWebSocketServer: z.unknown().optional().describe("@deprecated: use 'uiServer' instead"), useWorkerPool: z.boolean().optional().describe("@deprecated: use 'worker.processType' instead"), diff --git a/src/utils/index.ts b/src/utils/index.ts index 2cb7b800..bb57801e 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -19,6 +19,7 @@ export { LogConfigurationSchema, StationTemplateUrlSchema, StorageConfigurationSchema, + SupervisionUrlSchema, UI_SERVER_ACCESS_POLICY_DEFAULTS, UIServerConfigurationSchema, UIServerMetricsConfigurationSchema, diff --git a/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts b/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts index 70568179..c1ee0728 100644 --- a/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts +++ b/tests/charging-station/ui-server/ui-services/UIServiceRequestPayloadSchemas.test.ts @@ -252,6 +252,25 @@ await describe('UIServiceRequestPayloadSchemas', async () => { } }) + await it('should reject a supervision url whose scheme the WebSocket client cannot dial', async () => { + const { service } = createServiceContext(2) + + try { + for (const url of ['localhost:8010/OCPP16', 'ftp://localhost:8010/OCPP16']) { + assertRejectedWith( + await dispatchUntrustedPayload(service, ProcedureName.SET_SUPERVISION_URL, { + hashIds: [TEST_HASH_ID], + url, + }), + /url.*must be a WebSocket URL/ + ) + } + assert.strictEqual(service.getBroadcastChannelOutstandingResponseCount(TEST_UUID), 0) + } finally { + service.stop() + } + }) + await it('should accept a websocket supervision url', async () => { const { service } = createServiceContext(2) diff --git a/tests/utils/ConfigurationMigrations.test.ts b/tests/utils/ConfigurationMigrations.test.ts index ccb77145..842b39ce 100644 --- a/tests/utils/ConfigurationMigrations.test.ts +++ b/tests/utils/ConfigurationMigrations.test.ts @@ -30,7 +30,7 @@ await describe('ConfigurationMigrations', async () => { await describe('CURRENT_CONFIGURATION_SCHEMA_VERSION', async () => { await it('should be a positive integer', () => { assert.ok(Number.isInteger(CURRENT_CONFIGURATION_SCHEMA_VERSION)) - assert.strictEqual(CURRENT_CONFIGURATION_SCHEMA_VERSION, 1) + assert.strictEqual(CURRENT_CONFIGURATION_SCHEMA_VERSION, 2) }) }) @@ -305,6 +305,17 @@ await describe('ConfigurationMigrations', async () => { assert.strictEqual(result.foo, 'bar') }) + await it('should migrate a v1 configuration to CURRENT without changing its values', () => { + const result = applyConfigurationMigration( + 1, + { foo: 'bar', supervisionUrls: 'ws://localhost:8010/OCPP16' }, + 'test.json' + ) + assert.strictEqual(result.$schemaVersion, CURRENT_CONFIGURATION_SCHEMA_VERSION) + assert.strictEqual(result.foo, 'bar') + assert.strictEqual(result.supervisionUrls, 'ws://localhost:8010/OCPP16') + }) + await it('should not mutate the input config (immutability boundary)', () => { const input = { foo: 'bar' } const before = JSON.stringify(input) diff --git a/tests/utils/ConfigurationSchema.test.ts b/tests/utils/ConfigurationSchema.test.ts index 82959275..977c5d96 100644 --- a/tests/utils/ConfigurationSchema.test.ts +++ b/tests/utils/ConfigurationSchema.test.ts @@ -6,6 +6,7 @@ import assert from 'node:assert/strict' import { readFileSync } from 'node:fs' import { afterEach, describe, it } from 'node:test' +import { WebSocket } from 'ws' import { CURRENT_CONFIGURATION_SCHEMA_VERSION, @@ -13,6 +14,7 @@ import { } from '../../src/utils/index.js' import { ConfigurationSchema, + SupervisionUrlSchema, UI_SERVER_ACCESS_POLICY_DEFAULTS, WorkerConfigurationSchema, } from '../../src/utils/index.js' @@ -852,6 +854,83 @@ await describe('ConfigurationSchema', async () => { }) }) + await describe('SupervisionUrlSchema', async () => { + const acceptedUrls = [ + 'ws://localhost:8010/OCPP16', + 'wss://localhost:8010/OCPP16', + 'http://localhost:8010/OCPP16', + 'https://localhost:8010/OCPP16', + 'ws+unix:///tmp/simulator.sock', + 'ws+unix:///tmp/simulator.sock:/OCPP16', + 'ws:///OCPP16', + ] + const rejectedUrls = [ + 'localhost:8010/OCPP16', + 'ftp://localhost:8010/OCPP16', + 'file:///tmp/simulator.sock', + 'not-a-url', + '/relative', + ] + + await it('should accept every scheme the ws client can dial', () => { + for (const url of acceptedUrls) { + assert.strictEqual(SupervisionUrlSchema.safeParse(url).success, true, url) + } + }) + + await it('should reject values the ws client cannot dial with a message naming the accepted schemes', () => { + for (const url of rejectedUrls) { + const result = SupervisionUrlSchema.safeParse(url) + if (result.success) { + assert.fail(`Expected '${url}' to be rejected`) + } + assert.match(result.error.issues[0].message, /ws:\/\/, wss:\/\//) + } + }) + + await it('should reject a URL with a fragment identifier', () => { + const result = SupervisionUrlSchema.safeParse('ws://localhost:8010/OCPP16#fragment') + if (result.success) { + assert.fail('Expected a URL with a fragment identifier to be rejected') + } + assert.match(result.error.issues[0].message, /fragment identifier/) + }) + + await it('should reject a ws+unix URL without a socket path', () => { + for (const url of ['ws+unix://', 'ws+unix:']) { + const result = SupervisionUrlSchema.safeParse(url) + if (result.success) { + assert.fail(`Expected '${url}' to be rejected`) + } + assert.match(result.error.issues[0].message, /socket path/) + } + }) + + await it('should match the URLs accepted by the ws client', () => { + const edgeCaseUrls = [ + 'ws://localhost:8010/OCPP16#fragment', + 'wss://localhost:8010/OCPP16#', + 'ws+unix://', + 'ws+unix:', + ] + for (const url of [ + ...acceptedUrls, + ...rejectedUrls.filter(url => URL.canParse(url)), + ...edgeCaseUrls, + ]) { + let wsAccepts = true + try { + const webSocket = new WebSocket(url) + webSocket.on('error', () => undefined) + webSocket.terminate() + } catch { + wsAccepts = false + } + assert.strictEqual(SupervisionUrlSchema.safeParse(url).success, wsAccepts, url) + } + }) + }) + await describe('mixed-type fields', async () => { await it('should accept supervisionUrls as string', () => { const result = ConfigurationSchema.safeParse( @@ -869,6 +948,22 @@ await describe('ConfigurationSchema', async () => { assert.ok(result.success) }) + await it('should reject a scheme-less supervisionUrls string', () => { + const result = ConfigurationSchema.safeParse( + buildMinimalConfiguration({ supervisionUrls: 'localhost:8010/OCPP16' }) + ) + assert.strictEqual(result.success, false) + }) + + await it('should reject a supervisionUrls array entry with an unsupported scheme', () => { + const result = ConfigurationSchema.safeParse( + buildMinimalConfiguration({ + supervisionUrls: [TEST_SUPERVISION_URL, 'ftp://localhost:8010/OCPP16'], + }) + ) + assert.strictEqual(result.success, false) + }) + await it('should accept worker.elementsPerWorker as auto', () => { const result = ConfigurationSchema.safeParse( buildMinimalConfiguration({ worker: { elementsPerWorker: 'auto' } }) diff --git a/tests/utils/ConfigurationValidation.test.ts b/tests/utils/ConfigurationValidation.test.ts index b12833ef..866420b3 100644 --- a/tests/utils/ConfigurationValidation.test.ts +++ b/tests/utils/ConfigurationValidation.test.ts @@ -140,8 +140,10 @@ await describe('ConfigurationValidation', async () => { ) }) - await it('should accept string "$schemaVersion": "1" (no-migration path)', () => { - const parsed = buildMinimalConfiguration({ $schemaVersion: '1' }) + await it('should accept string "$schemaVersion" equal to CURRENT (no-migration path)', () => { + const parsed = buildMinimalConfiguration({ + $schemaVersion: CURRENT_CONFIGURATION_SCHEMA_VERSION.toString(), + }) const result = validateConfiguration(parsed, 'string-version.json') @@ -151,6 +153,36 @@ await describe('ConfigurationValidation', async () => { ) }) + await it('should migrate a valid v1 configuration to the current schema version', () => { + const parsed = buildMinimalConfiguration({ + $schemaVersion: 1, + supervisionUrls: 'ws://localhost:8010/OCPP16', + }) + + const result = validateConfiguration(parsed, 'v1.json') + + assert.strictEqual( + (result as unknown as Record).$schemaVersion, + CURRENT_CONFIGURATION_SCHEMA_VERSION + ) + assert.strictEqual(result.supervisionUrls, 'ws://localhost:8010/OCPP16') + }) + + await it('should report a v1 supervisionUrls value the ws client cannot dial', () => { + const parsed = buildMinimalConfiguration({ + $schemaVersion: 1, + supervisionUrls: 'localhost:8010/OCPP16', + }) + + assert.throws( + () => validateConfiguration(parsed, 'v1-invalid.json'), + (error: unknown) => + error instanceof ConfigurationValidationError && + error.migratedFrom === 1 && + error.fieldErrors.some(fieldError => fieldError.path === 'supervisionUrls') + ) + }) + await it('should sweep deprecated keys unconditionally for v1 configs', t => { t.mock.method(console, 'warn', () => undefined) const parsed = buildV1WithDeprecatedKey('workerPoolSize', 16) @@ -482,7 +514,7 @@ await describe('ConfigurationValidation', async () => { await describe('error message snapshot', async () => { await it('should match error message snapshot', () => { try { - validateConfiguration({ $schemaVersion: 1 }, 'test.json') + validateConfiguration({ $schemaVersion: CURRENT_CONFIGURATION_SCHEMA_VERSION }, 'test.json') assert.fail('Expected ConfigurationValidationError') } catch (error) { assert.ok(error instanceof ConfigurationValidationError) -- 2.53.0