Skip to content

source-control:babysit-prs: the merge wrapper's --allow-unpinned-head refusal is bypassable by argparse prefix abbreviation #1371

Description

@kyle-sexton

Problem

bin/source-control-babysit-merge exists to add exactly one refusal on top of babysit_merge.py. Its own header states the purpose:

It additionally refuses --allow-unpinned-head so that no unattended, allow-rule-covered invocation can merge an unvetted head; the interactive unpinned override remains available by invoking babysit_merge.py directly.

That refusal is bypassable by dropping one character from the flag.

Root cause — two halves that disagree

The wrapper filters by exact string equality (bin/source-control-babysit-merge:15-22 on origin/main):

for arg in "$@"; do
  if [[ "$arg" == "--allow-unpinned-head" ]]; then
    printf '%s\n' \
      'source-control-babysit-merge: --allow-unpinned-head is not permitted through the wrapper.' \
      'Invoke babysit_merge.py directly for interactive unpinned use.' >&2
    exit 2
  fi
done

The CLI it wraps builds its parser with argparse's prefix abbreviation left at the default (skills/babysit-prs/scripts/babysit_merge.py:806):

parser = argparse.ArgumentParser(description=__doc__)   # allow_abbrev defaults to True

So any unambiguous prefix — --allow-unpinned-hea, --allow-unpinned-h, --allow-unp — is a valid spelling of the flag to the CLI and an unrecognized string to the wrapper.

Reproduction (on origin/main; both files are byte-identical to main on every open branch checked)

Exact spelling — the guard fires:

$ bash bin/source-control-babysit-merge --allow-unpinned-head
source-control-babysit-merge: --allow-unpinned-head is not permitted through the wrapper.
Invoke babysit_merge.py directly for interactive unpinned use.
RC=2

Abbreviated — the guard does not fire, and argparse accepts the flag rather than reporting an unrecognized argument. Execution proceeds all the way to the owner-allowlist scope check:

$ bash bin/source-control-babysit-merge melodic-software/claude-code-plugins#1285 --allow-unpinned-hea
{"pr": "melodic-software/claude-code-plugins#1285", "inScope": false, "error": "--allowed-owners is required and must be non-empty; refusing to act without an owner allowlist"}
RC=3

RC=3 from the scope check, not RC=2 from the wrapper, is the proof: the argument was consumed as a recognized option. Had argparse rejected it, the failure would have been unrecognized arguments.

Impact

babysit_merge.py:1051 is what the override unlocks:

if not args.expected_head and not args.allow_unpinned_head:

With --merge and a valid --allowed-owners, an invocation that reads as the guarded wrapper form can therefore merge a pull request with no pinned head — the precise outcome the wrapper's existence is justified by. Any consumer whose permission rule is anchored on the wrapper's bare name (which is what the wrapper is packaged for) grants that invocation.

Severity is bounded, not zero, and the bound is a consumer's configuration rather than this repository's code:

  • A consumer whose allow rule covers the wrapper only without --merge is not straight-through exploitable, because the merging invocation is judged on its own terms. That is how melodic-software/dotfiles' autoMode.allow is currently written.
  • A consumer who grants the wrapper broadly — on the documented reasoning that the wrapper cannot merge an unvetted head — is exposed.

Either way the shipped claim is false, and the defense-in-depth layer the wrapper is named for does not hold.

Suggested fix

Both halves, because either alone leaves a gap:

  1. babysit_merge.py:806argparse.ArgumentParser(description=__doc__, allow_abbrev=False). This is the root cause and it closes the class, not just this flag: every other option on this parser is equally abbreviatable today, and a future wrapper-side filter would inherit the same hole. Check the other eight lane parsers for the same default before assuming this one is special.
  2. bin/source-control-babysit-merge:15-22 — match the flag as a prefix and its --flag=value form rather than by string equality, so the wrapper refuses independently of what the CLI happens to accept. A guard that is correct only because the thing it guards is configured a particular way is not a guard.

Then bind it: the guard contract in #1265 / #1285 publishes merge.unpinned-head-refused-by-wrapper as a proven claim, and no assertion in the suite currently exercises an abbreviated spelling. A row asserting that every prefix of the flag is refused is what keeps this closed.

Provenance

Found by the Codex reviewer on #1285 (guard_contract.py:301, 2026-07-25T07:14:47Z) and independently confirmed empirically here against origin/main before filing. Not introduced by #1285 — that PR only publishes a contract row claiming the refusal holds.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.priority: highSignificant impact, or blocks an imminent release; staff this cycle.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions