fix(beacon): narrow TOCTOU window in handlePunchRequest (PILOT-339)

[matthew-pilot]

Summary

PILOT-339: In handlePunchRequest, the target and requester endpoints are snapshot once and then used for SendPunchCommand after a delay (the second send). Between the snapshot and the send, the peer may have re-registered from a different NAT binding, causing a TOCTOU stale-address → lost punch → daemon retry.

This fix re-snapshots each participant's address immediately before the corresponding SendPunchCommand call, narrowing the staleness window significantly. The underlying race remains possible (no locking across the UDP write) — UDP best-effort absorbs the remaining loss; the daemon retries on timeout.

Changes

• server.go: inline re-snapshot of targetID and requesterID before each send in handlePunchRequest (+ documentation of the trade-off)

Verification

go build, go vet, and relevant tests (TestHandlePunchRequest_*, TestSendPunchCommand_*, TestNodeMap_*) all pass.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[matthew-pilot]

🦭 Matthew PR Check — #13 PILOT-339

Status

• State: OPEN · MERGEABLE ✅
• CI: 2/2 green (test ✅, codecov/patch ✅)
• Files: 1 (+16 −1) — server.go
• Tier: small (≤3 files, ≤50 LoC)
• Canary: not configured for beacon

Verdict

CLEAN — all CI green, mergeable, no review conflicts. Small focused fix narrowing TOCTOU window in handlePunchRequest. Ready for operator review.

[matthew-pilot]

🦭 Matthew Explains — #13 PILOT-339

What this does

In handlePunchRequest, the target and requester endpoints were snapshot once at the top of the function and then used for SendPunchCommand after a delay (the second send call). Between the snapshot and the send, the peer may have re-registered from a different NAT binding, causing a TOCTOU stale-address → lost punch → daemon retry.

This fix re-snapshots each participant's address immediately before the corresponding SendPunchCommand call, narrowing the staleness window significantly.

Why it's safe

• No locking needed — the underlying race (peer re-registering between snapshot and UDP write) is fundamentally unavoidable without a lock spanning the entire UDP send. UDP is best-effort by design; the daemon retries on timeout if the punch is lost.
• 16-line change, single file, all tests pass.
• No wire-format change — this is purely an internal timing improvement.

What to look for in review

• The re-snapshot happens inside the per-participant SendPunchCommand call path, not globally — the two participants may still be at different staleness levels if one re-registered between the two sends. This is documented and accepted.

[matthew-pilot]

🧹 Matthew Cleanup — #13 Merged

PR merged by TeoSlayer at 2026-05-29T20:36:19Z. Cleaning up now.

• Branch: openclaw/pilot-339-20260529-200700 — will delete
• Merge commit: 8115fcd6
• Jira: PILOT-339 → Done

Thanks for the merge! 🚀

[matthew-pilot]

✅ Matthew Merged Cleanup — #13 PILOT-339

Merged by TeoSlayer at 2026-05-29 20:36 UTC

Branch openclaw/pilot-339-20260529-200700 auto-deleted on merge.

Action: Jira PILOT-339 → READY. The TOCTOU window in handlePunchRequest has been narrowed. UDP best-effort absorbs any remaining race; daemon retries on timeout.