fix(ocpp): periodic dispatch, Lua allowlist, HTTP timeouts - #1176
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8109e0f5-1647-447f-97ce-18da53aec84f) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Merge note (no reviewer ping): this PR also edits
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11d644adf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| func (r evCommandRouter) SendCycle(ctx context.Context, name string, payload []byte, cycleID uint64) error { | ||
| if r.ocppRoute(name) { | ||
| return r.ocpp.Command(ctx, name, payload) |
There was a problem hiding this comment.
Preserve the cycleID sequencing when routing OCPP pause/resume commands instead of calling ocpp.Command directly. If a wallbox cycle pauses and a subsequent tick sends 0 W during the three-second gap—particularly because the site meter became stale—the later ev_resume still runs and restores Handler.LastAmps; unlike Registry.SendEVContinuation, this path cannot detect that the intervening command superseded the cycle, so charging can restart after the safety standdown.
AGENTS.md reference: AGENTS.md:L34-L35
Useful? React with 👍 / 👎.
| httpReadTimeout = 15 * time.Second | ||
| // WriteTimeout must outlast assistant.Timeout (90s): Ask why streams SSE | ||
| // on this listener, and a shorter write ceiling would cut the reply off. | ||
| httpWriteTimeout = 2 * time.Minute |
There was a problem hiding this comment.
Avoid imposing this absolute two-minute write deadline on the backup endpoints. handleBackupCreate performs the complete archive synchronously before writing its JSON response, and handleBackupDownload streams the archive with http.ServeFile; on a large state database or a slow connection, either operation can exceed two minutes, causing creation to finish without a response or downloads to be truncated. Use handler-specific deadlines or otherwise exempt these long-running responses while retaining bounded deadlines for ordinary API calls.
Useful? React with 👍 / 👎.
Periodic loadpoint ticks sent ev_set_current only through the Lua registry, so an online OCPP charger never saw planner, surplus, or hold commands. Route sendOutcome and sendCycle through the same OCPP-aware sender as the API. Approved OCPP identities are loadpoint names that are not Lua drivers, plus ocpp.chargers ids, so a shared secret cannot impersonate Easee. Bound HTTP Read/Write/Idle timeouts and fail oversized JSON at the cap. Closes #1152 Closes #1157 Closes #1137 Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
11d644a to
55549bb
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b972b531-58dc-447b-8625-15552059e97b) |
1 participant
Periodic OCPP dispatch never reached the charger, Lua loadpoint names were treated as charge-point identities, and the HTTP server only capped header time.
#1152 periodic dispatch
SetOutcomeSender(reg.SendWithOutcome)/SetCycleSender(reg.SendEVContinuation)bypassed the OCPP-awareevSendused by/api/ev/command. Periodicev_set_currenttherefore died asdriver "…" not foundfor any charger that is not in the Lua registry.evCommandRouternow owns Send, SendWithOutcome, and SendCycle. A controller withSetOutcomeSenderand an online adopted OCPP name callsocpp.Command.#1157 Lua names on the allowlist
Approved IDs were every loadpoint
DriverName. Connecting aseaseewith the shared secret was therefore adopted, wrote DerEV, and stole commands.ocppApprovedIDsnow keeps Lua driver names off the allowlist. Identities that are actually OCPP loadpoints, plusocpp.chargersids, stay approved. A WebSocket to/easeestays pending and does not receiveev_set_current.#1137 HTTP timeouts
Boot and main listeners now set
ReadTimeout(15s),WriteTimeout(2m, longer than Ask why SSE), andIdleTimeout(60s).readJSONuseshttp.MaxBytesReaderand fails at the 1 MiB cap instead of unmarshalling a truncated prefix.Closes #1152
Closes #1157
Closes #1137
Note
High Risk
Changes OCPP identity adoption, EV command routing, and periodic dispatch—security-sensitive paths where mis-routing could block charging or allow impersonation; HTTP timeout tweaks are lower risk but affect all API traffic including long SSE.
Overview
Fixes three production issues around OCPP EV control and HTTP hardening.
Periodic dispatch to OCPP chargers — Planner ticks used
SetOutcomeSender/SetCycleSenderwired straight to the Lua registry, soev_set_currentfailed withdriver "…" not foundfor wallboxes that only exist over OCPP. A sharedevCommandRouternow routesSend,SendWithOutcome, andSendCyclethroughocpp.Commandwhen the name is online and approved, matching/api/ev/command. OCPP “online” for dispatch and telemetry now also requiresIsApproved, not just a live WebSocket.OCPP allowlist — Approved charge-point IDs were every loadpoint
driver_name, so someone with the shared basic-auth secret could connect as a Lua driver name (e.g.easee), get adopted, inject DerEV, and receive commands.ocppApprovedIDsdrops names that map to Lua drivers; only explicitocpp.chargersIDs and loadpoints whose driver is not a Lua driver stay approved. Impostor connections stay pending and do not get current commands over OCPP.HTTP — Boot and API listeners use
newHTTPServerwith read (15s), write (2m, above Ask/SSE), and idle (60s) timeouts. APIreadJSONuseshttp.MaxBytesReaderat 1 MiB so oversized bodies error instead of unmarshaling a truncated prefix.Reviewed by Cursor Bugbot for commit 55549bb. Bugbot is set up for automated code reviews on this repo. Configure here.