shim: guard PEERAGENT_BIN against self-exec loop (pointing at a shim) - #2
Merged
Merged
Conversation
…xec loop) PEERAGENT_BIN is meant to override the resolved Go binary. If a user sets it to a shim copy (this script, or plugin/bin/peeragent), step 1 does `exec "$PEERAGENT_BIN" "$@"` -> the shim re-runs step 1 with the same env var -> exec itself -> forever. Symptom: a `sh` process pinning a core with no child, spotted in the wild this session (an opus peer-review job that "ran" for 19min without ever starting the agent). Guard: in step 1, if the PEERAGENT_BIN target's first two bytes are `#!` (a shell shim; the Go binaries start with ELF/Mach-O magic), fail fast with exit 2 and a clear message instead of spinning. Catches pointing at ANY shim copy (root bin/ or plugin/bin/), not just the exact self path. A valid override to the Go binary and the unset/auto-detect path are unaffected (verified: shim->self and shim->plugin-shim now exit 2; shim->Go-binary and unset both still work). The --text flag is detected inline (wants_text is defined later in the script and sh does not parse functions ahead, so it can't be called from step 1). Re-synced plugin/bin/peeragent from bin/peeragent via package-plugin.sh. Test: added a "shim PEERAGENT_BIN self-exec-loop guard" step to scripts/validate.sh — runs the guarded path under `timeout 3` so a regression (hang/loop) fails the suite rather than passing by luck. Asserts process exit 2 (set by the shim, independent of the Go binary's exit-code semantics) and the "self-exec loop" message. Note (out of scope, flagged): the pre-existing "shim smoke" step asserts process exit 4 for `--status missing-job`, but the Go binary's process exit is 1 (the `exit_code:4` lives only in the JSON metadata) on this build — that step appears already red on clean upstream, unrelated to this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PEERAGENT_BINis meant to override the resolved Go binary. If set to a shim copy (this script, orplugin/bin/peeragent), step 1 runs:→ the shim re-enters step 1 with the same env var →
execitself → infinite self-exec loop. It burns a core and never starts the agent. No error, no exit.Spotted in the wild this session: an opus
peer-reviewjob reported "running" for 19 minutes while a bareshprocess sat at 53% CPU with no child — the review never actually executed. Easy footgun: the natural but wrong instinct isPEERAGENT_BIN=$(which peeragent)pointing at the shim.Fix
In step 1, if the
PEERAGENT_BINtarget's first two bytes are#!(a shell shim — the Go binaries start with ELF/Mach-O magic), fail fast with exit 2 and a clear message instead of spinning.Catches pointing at any shim copy (root
bin/orplugin/bin/), not just the exact self path.--textis detected inline becausewants_textis defined later in the script andshdoes not parse functions ahead (verified: calling a later-defined function from step 1 givescommand not found).Verified
PEERAGENT_BIN→ shim itself (the bug)PEERAGENT_BIN→ other shim copy (plugin/bin/peeragent)PEERAGENT_BIN→ real Go binaryPEERAGENT_BINunset → auto-detect--textmodeset -euTest
Added a "shim PEERAGENT_BIN self-exec-loop guard" step to
scripts/validate.sh— runs the guarded path undertimeout 3so a regression (guard removed/broken) hangs and fails the suite rather than passing by luck. Asserts process exit 2 (set by the shim itself, independent of the Go binary's exit-code semantics) and theself-exec loopmessage.plugin/bin/peeragentre-synced frombin/peeragentviapackage-plugin.sh.Out of scope (flagged, not fixed here)
The pre-existing "shim smoke" step asserts process exit 4 for
--status missing-job, but on this build the Go binary's process exit is 1 (theexit_code:4lives only in the JSON metadata). That step appears already red on clean upstream, unrelated to this change. Worth a separate look at how the Go binary maps failure status → process exit code.