fix(ui-server): validate UI request payloads against a canonical schema (#2129)
* 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<ProcedureName, ZodType>).
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