fix a long-running CI bug that overran a lot (#10707)
* fix the fix * Address Copilot review: add EXIT trap and clarify PKC comment Add `trap` to kill meshtasticd on any early exit (python harness failure, socket timeout) so CI never leaks a background process. Reword the ARCH_PORTDUINO comment to make explicit that pki_encrypted=true causes the from==0 plain-admin branch to be skipped, routing into the PKC key-check — the underlying logic was correct all along. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update PORTDUINO comment to reflect from==0 auth fix The from==0 branch no longer requires !pki_encrypted (fixed upstream in this branch), so update the simulator comment to reflect the actual remaining reason for the early intercept: is_managed could still block exit_simulator even for local packets. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
GitHub
Claude Sonnet 4.6
parent
b76e5e6ba4
commit
b938b63e8a
@@ -37,13 +37,33 @@ jobs:
|
||||
sed -i -e "s#${PWD}#.#" coverage_base.info # Make paths relative.
|
||||
|
||||
- name: Integration test
|
||||
# Cap the whole step: if the simulator ever fails to exit (e.g. the
|
||||
# exit_simulator admin path regresses again) the job must fail fast,
|
||||
# not run to GitHub's 6-hour limit.
|
||||
timeout-minutes: 5
|
||||
run: |
|
||||
.pio/build/coverage/meshtasticd -s &
|
||||
PID=$!
|
||||
trap 'kill "$PID" 2>/dev/null || true' EXIT
|
||||
timeout 20 bash -c "until ls -al /proc/$PID/fd | grep socket; do sleep 1; done"
|
||||
echo "Simulator started, launching python test..."
|
||||
python3 -c 'from meshtastic.test import testSimulator; testSimulator()'
|
||||
wait
|
||||
# The Python harness sends exit_simulator and exits; the simulator is
|
||||
# expected to terminate on its own. Give it a moment, then verify.
|
||||
# If it is still alive the exit handshake is broken — fail loudly and
|
||||
# do NOT fall through to `wait`, which would otherwise block until the
|
||||
# job's hard timeout.
|
||||
for i in $(seq 1 10); do
|
||||
kill -0 "$PID" 2>/dev/null || break
|
||||
sleep 1
|
||||
done
|
||||
if kill -0 "$PID" 2>/dev/null; then
|
||||
echo "::error title=Simulator did not exit::meshtasticd ignored exit_simulator and is still running after the integration test. The exit_simulator admin path is broken (see AdminModule::handleReceivedProtobuf, ARCH_PORTDUINO bypass). Killing it to avoid a 6-hour CI overrun."
|
||||
kill -9 "$PID" 2>/dev/null || true
|
||||
wait "$PID" 2>/dev/null || true
|
||||
exit 1
|
||||
fi
|
||||
wait "$PID" 2>/dev/null || true
|
||||
|
||||
- name: Capture coverage information
|
||||
if: always() # run this step even if previous step failed
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
#include "main.h"
|
||||
#endif
|
||||
#ifdef ARCH_PORTDUINO
|
||||
#include "PortduinoGlue.h"
|
||||
#include "unistd.h"
|
||||
#endif
|
||||
|
||||
@@ -106,6 +107,18 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta
|
||||
if (mp.which_payload_variant != meshtastic_MeshPacket_decoded_tag) {
|
||||
return handled;
|
||||
}
|
||||
#ifdef ARCH_PORTDUINO
|
||||
// Simulator only: honor exit_simulator unconditionally for the local client (from==0).
|
||||
// The from==0 branch below now covers pki_encrypted local packets too, but is_managed
|
||||
// can still block it. Rather than threading simulator awareness through the auth gates,
|
||||
// intercept here before any auth logic runs. Local-origin + force_simradio only.
|
||||
// TODO: should a local client bypass admin auth at all? Fenced to the simulator for now.
|
||||
if (portduino_config.force_simradio && mp.from == 0 &&
|
||||
r->which_payload_variant == meshtastic_AdminMessage_exit_simulator_tag) {
|
||||
LOG_INFO("Exiting simulator");
|
||||
exit(0);
|
||||
}
|
||||
#endif
|
||||
meshtastic_Channel *ch = &channels.getByIndex(mp.channel);
|
||||
// Could tighten this up further by tracking the last public_key we went an AdminMessage request to
|
||||
// and only allowing responses from that remote.
|
||||
|
||||
Reference in New Issue
Block a user