Bound CommonCLI replies at the 157 bytes every caller can give (iteration branch)

[ptr727]

Iteration branch for #25. Keep it open; never merge it. A clean single-commit branch for upstream is cut from it once it has baked. Pieter's decision on 2026-10-10: nothing goes upstream until a maintainer engages with one of our open PRs.

References:

• The code:
  • before: upstream dev 2dbd463e
  • after: 024254a4
• The measurement: cli-reply-bound-harness.cpp, a host transcription of the writers and the wrapper, with inputs constructed in the program.

The defect

The repeater, room server and sensor wrappers echo an optional 3-byte xx| prefix into the reply and advance reply past it before calling CommonCLI::handleCommand(). A serial caller's char reply[160] and a remote caller's &temp[5] of uint8_t temp[166] therefore leave CommonCLI 157 and 158 bytes. Several of its writers bound themselves at 160 instead. #25 has the permalinks.

The fix

• CommonCLI.h L25-L28 adds CLI_REPLY_SIZE (157), with the derivation in a comment.
• Every reply bound in CommonCLI.cpp now uses it: L272, L294, L376-L380, L697, L735, L872-L881, L897, L903, L910 and L1020. The change only replaces the literal, so the line numbers match dev.
• A reply without the prefix loses at most its last 3 characters.

Evidence

Measured with the harness. Each command is at most 158 characters, which is what a serial command[160] keeps:

Command	Bound	Serial reply[160]	Remote temp[166]
xx|set + a 151-character key	160 (dev)	2 bytes past the end	1 byte past the end
xx|set + a 151-character key	157 (this PR)	0	0
xx|region def + a 144-character name with a .	160 (dev)	3 bytes past the end	2 bytes past the end
xx|region def + a 144-character name with a .	157 (this PR)	0	0
xx|get owner.info, owner_info full (control)	160 or 157	0	0

Testing

• Build: RAK_4631_repeater, RAK_4631_room_server, RAK_4631_sensor and heltec_v4_r8_repeater all build at 024254a4.
• Host: the harness above.
• Hardware: not run. The overrun is a stack write the device won't report, so a board would show nothing the harness doesn't. The only test board is held on the RV3028 acceptance build.

Notes

• Not changed: the sensor list and neighbours loops, which stop adding entries at a fixed 134 bytes. Upstream Reject a negative sensor list start index 🤖🤖 meshcore-dev/MeshCore#3433 left the sensor list bound as it is, and the neighbours loop isn't audited here.
• Parity: ZephCore fixed the same overrun in liquidraver/ZephCore@ad23dcb. It subtracts the prefix from its separate remote bound. MeshCore has a single bound, so one constant covers both paths.

Refs #25

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: bfe25dce-7040-4769-b5f8-f9899ee2ed54

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟢 Approval recommended

The changed bounds match the smallest caller capacity; the remaining documentation nit is non-blocking.

0 open findings

What changed in this PR

This PR limits CommonCLI replies to the 157 bytes available after a caller echoes an optional prefix, addressing the buffer overrun described in #25.

Changes:

• Defines a shared reply-size limit.
• Applies it to reply writers, including GPS diagnostics and region commands.

File	Description
src/​helpers/​CommonCLI.h	Defines and explains the 157-byte limit.
src/​helpers/​CommonCLI.cpp	Uses the limit for affected reply bounds.

🧠 Review effort: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.