Add MSP configuration parser for Hayward OmniLogic local binding#65
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
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, or 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 fix this CI failure" or "@codex address that feedback".
| static { | ||
| XSTREAM.ignoreUnknownElements(); | ||
| XSTREAM.addPermission(AnyTypePermission.ANY); | ||
| XSTREAM.processAnnotations(MspConfig.class); | ||
| } |
There was a problem hiding this comment.
[P1] Register annotations for nested MSP config types
The parser only calls XSTREAM.processAnnotations(MspConfig.class), so XStream never processes the @XStreamAsAttribute and @XStreamImplicit annotations defined on SystemConfig, BackyardConfig, PumpConfig, etc. When parsing real XML (like the snippet in ConfigParserTest), those annotations are ignored and the collections stay empty while attributes such as systemId remain null. Either pass all involved classes to processAnnotations or enable annotation autodetection so the model is populated correctly.
Useful? React with 👍 / 👎.
| static { | ||
| XSTREAM.ignoreUnknownElements(); | ||
| XSTREAM.addPermission(AnyTypePermission.ANY); | ||
| XSTREAM.processAnnotations(MspConfig.class); |
There was a problem hiding this comment.
[P1] Remove unrestricted XStream AnyTypePermission
Granting AnyTypePermission.ANY disables XStream’s security model and allows deserialization of arbitrary classes. If the MSP configuration XML ever comes from a compromised device or from the network, this creates an RCE surface in the binding. Restrict the allowed types to the concrete config classes instead of granting global permission.
Useful? React with 👍 / 👎.
Summary
Testing
mvn -q -pl bundles/org.openhab.binding.haywardomnilogiclocal -am test(fails: Non-resolvable parent POM)https://chatgpt.com/codex/tasks/task_e_68c639bb32f48323be074c5299404193