fix(teslemetry): decode dotted-quad IPv4 gateway addresses with strict validation - #127
Conversation
…decode networking_status now serves ipv4_config address/subnet_mask/gateway as dotted-quad strings on some sites, alongside the legacy big-endian uint32 int; _decode_ipv4 only handled the int form, so find_gateway_address() silently returned None on affected sites.
…validation, fix docstring
![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: 96b39c0e61
ℹ️ 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".
| return None | ||
| address = socket.inet_ntoa(struct.pack(">I", value)) | ||
| elif isinstance(value, str): | ||
| if not _DOTTED_QUAD_RE.match(value): |
There was a problem hiding this comment.
When an address string ends in \n, Python's $ anchor matches immediately before that final newline, so _DOTTED_QUAD_RE.match() succeeds and the unvalidated original string is returned. In particular, "0.0.0.0\n" also bypasses the sentinel equality check and can shadow a valid Wi-Fi address during interface selection. Use fullmatch() or a strict end-of-string anchor so any surrounding whitespace remains undecodable.
AGENTS.md reference: AGENTS.md:L149-L149
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: switched to fullmatch() in 3003ebf, closing the trailing-newline/whitespace gap for both a valid address and the two sentinels. Added regression tests for both cases.
re.match with a $ anchor accepts a trailing newline, so a string like "0.0.0.0\n" passed the regex and then failed the exact sentinel equality check, letting a malformed/undecodable address slip through as if it were a validated one. fullmatch requires the whole string to match, closing that gap.
Bre77
changed the title
1 participant
Intent
Fix a Codex PR-review-flagged P2 on PR #127 (fm/pyfleet-decode-ipv4-string): _DOTTED_QUAD_RE.match() used a $ anchor, which Python regex matches before a trailing newline, so a malformed string like '0.0.0.0\n' passed the regex and then bypassed the exact '0.0.0.0'/'255.255.255.255' sentinel equality check - it could shadow a valid address in interface selection instead of being treated as undecodable. Fixed by switching match() to fullmatch(), which requires the entire string to match with no unmatched trailing characters. Added regression tests for trailing-newline and surrounding-whitespace forms of both a normal address and both sentinel values, including one exercising the actual fallback-to-wifi selection behavior when eth's address is a trailing-newline sentinel variant. All prior task requirements (dual uint32/string decoding, active_route preference, 0.0.0.0/255.255.255.255/bool rejection, None-on-malformed) remain unchanged and covered. Verified: ruff check/format, pyright strict, and full pytest suite (646 tests) all pass locally.
What Changed
find_gateway_address()intesla_fleet_api/teslemetry/energysite.pynow decodesnetworking_status.ipv4_config.addressas either a raw big-endian uint32 or a dotted-quad string (the API has been observed serving both forms), in addition to the existing uint32-only path.fullmatch()(notmatch()), so malformed input such as a trailing newline can no longer slip past the format check and falsely satisfy the0.0.0.0/255.255.255.255sentinel-rejection logic.docs/teslemetry.mdto describe the dual uint32/string decoding instead of the stale uint32-only description.tests/test_teslemetry_gateway_address.pycovering dotted-quad string decoding, trailing-newline and surrounding-whitespace malformed variants for both normal and sentinel addresses, and the fallback-to-wifi selection path when eth's address is an invalid sentinel variant.Risk Assessment
✅ Low: Single-line regex-method fix (match→fullmatch) verified by hand to correctly reject the trailing-newline/whitespace bypass described in the intent, backed by thorough new regression tests; no other functional code changed.
Testing
Ran the targeted gateway-address test file (26 tests) against the fix commit — all pass. To validate the tests aren't vacuous, I temporarily reverted the one-line fullmatch->match change and reran the new trailing-newline/whitespace tests: they failed exactly as the P2 review described ('0.0.0.0\n' and '255.255.255.255\n' slipping past the sentinel check, with the zero-sentinel case wrongly returning the malformed value instead of falling back to wifi's real address), confirming the fix and its regression coverage are effective. Working tree was restored to clean afterward.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
uv run pytest tests/test_teslemetry_gateway_address.py -v(26 passed) on target commit 3003ebfManually reverted.fullmatch()back to.match()in tesla_fleet_api/teslemetry/energysite.py and reran the new regression tests (-k "trailing_newline or surrounding_whitespace") to confirm they fail against the pre-fix behavior — 3 failed as expected, proving the tests actually exercise the fixed bugRestored the source file viagit checkout --and reran the full targeted test file to confirm 26/26 pass on the real fix✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.