merge-guard: value-carrying privileged-flag (`--repo`/`-R`) approval over-blocks — mint glues the option's closing backtick onto the flag value, breaking the #1042 set-equality bind (#1042 follow-up)

[michael-wojcik]

Summary

A merge approved via AskUserQuestion whose embedded command carries a value-carrying privileged flag as the last token — e.g.

gh pr merge 1058 --squash --delete-branch --repo Synaptic-Labs-AI/PACT-Plugin

is refused at execution time with:

An authorization token exists but its approved operation does not match this command. The approved command and the executed command must be identical.

…even though the executed command is byte-identical to the approved one. The operator recovers only by re-running the minimal command gh pr merge 1058 --squash (dropping the privileged flags), which authorizes cleanly.

This is the read-side mirror of #1052's mint-side ergonomics theme, but it is a structurally distinct, mint-succeeded-then-mismatch case — see "Why a sibling of #1052" below.

Root cause (verified live against the shipped modules)

The mint and read sides extract bound_flags from differently-delimited input, and the flag tokenizer strips no delimiters:

• MINT (merge_guard_post.py _mint_context_from_bundle, ~L381-391) resolves the command region cleanly via locate_command_regions, but then calls extract_command_context(..., flag_scan_text=" ".join(selected_option_texts)). That flag_scan_text is the full selected-option prose surface — Merge PR 1058 Run `gh pr merge 1058 --squash --delete-branch --repo Synaptic-Labs-AI/PACT-Plugin` — with the markdown backtick delimiters still attached (this widened scan was introduced by security: --admin and other privileged flags ride past merge-guard auth binding #1042 for the "flag after a quoted arg" symmetry case).
• extract_privileged_flags (shared/merge_guard_common.py, L495-626) tokenizes with a bare command.split() (L552) and never strips a surrounding quote/backtick from a value. Because --repo is value-carrying and its value is the last token inside the backtick-quoted literal, .split() glues the closing backtick onto the value and the value-capture branch (L585-586) emits `--repo=Synaptic-Labs-AI/PACT-Plugin``.
• READ (merge_guard_pre.py _token_matches_command, L1025-1093) scans the bare executed command (no flag_scan_text, no delimiters), yielding the clean --repo=Synaptic-Labs-AI/PACT-Plugin.

The two bound_flags sets then differ by exactly one trailing backtick, and the #1042 never-escalate axis (a2, L1074: set(context["bound_flags"]) != set(cmd["bound_flags"]) -> return False) refuses. operation_type and pr_number are identical on both sides; --delete-branch (boolean, not the last token) is identical; --squash is not privileged. The minimal command worked because it carries zero privileged flags, collapsing both sets to empty (trivially equal).

MINT: {'operation_type':'merge','pr_number':'1058',
       'bound_flags':['--delete-branch','--repo=Synaptic-Labs-AI/PACT-Plugin`']}  <- stray backtick
READ: {'operation_type':'merge','pr_number':'1058',
       'bound_flags':['--delete-branch','--repo=Synaptic-Labs-AI/PACT-Plugin']}   <- clean

Trigger (general): any value-carrying privileged flag (--repo/-R, --match-head-commit) — or a trailing boolean privileged flag like --admin (which becomes `--admin``, fails the exact denylist lookup at L569, and is silently dropped from the mint set, also yielding a mismatch) — positioned as the LAST token before the closing markdown backtick that delimits the embedded command literal.

Minimal repro (shipped functions)

from shared.merge_guard_common import extract_privileged_flags
extract_privileged_flags("--repo X`", "merge")   # MINT  -> ['--repo=X`']
extract_privileged_flags("--repo X",  "merge")   # READ  -> ['--repo=X']
# ['--repo=X`'] != ['--repo=X']  -> #1042 set-equality gate REFUSES

Safety classification: OVER-BLOCK-ONLY (the #1042 bind is doing its job)

This is an ergonomics / legibility bug in the safe #1031 direction — NOT a security/under-block. The read side authoritatively recomputes bound_flags from the actual executed command and refuses on ANY set difference in EITHER direction. The backtick artifact only ever corrupts the MINT (token) side — appending a stray char to a value or dropping a mangled flag. Neither can let the executed command carry MORE privilege than was approved while still matching: an execution-time escalation (an extra privileged flag in the real command) makes the read set a strict superset of the token set → still REFUSES. The never-escalate invariant is preserved; the only failure is REFUSING a legitimate, truly-identical command. Tokens also expire in 5 min (TOKEN_TTL=300), bounding any residual. The fix must not weaken the bind — it must make the two sides tokenize identical input.

Why a sibling of #1052, not a fold

Kinship is real (shared read-vs-mint symmetry / self-teaching spirit; #1052 is the mint-side mirror), but four axes make it a distinct sibling rather than a #1052 sub-case:

1. Different silence state / code path. merge-guard mint ergonomics: silent no-mint on format mismatch + multi-prompt approval (UX hardening) #1052's advisory fires in merge_guard_post.py main() ONLY when _mint_context_from_bundle returns context-is-None (no token minted). Here a token DID mint (context is not None) — just with a corrupted value — so merge-guard mint ergonomics: silent no-mint on format mismatch + multi-prompt approval (UX hardening) #1052's advisory provably never reaches this case. The denial happens later in merge_guard_pre.py _token_matches_command (L1074) → deny branch (L1144-1157).
2. Already non-silent. That read-side path already returns a message, so merge-guard mint ergonomics: silent no-mint on format mismatch + multi-prompt approval (UX hardening) #1052's "non-silent mint failure" AC does not apply. The gap here is axis-naming + the underlying tokenization bug, a different remediation than merge-guard mint ergonomics: silent no-mint on format mismatch + multi-prompt approval (UX hardening) #1052's add-a-message work.
3. Different files, including PR A's. The legibility half lives in merge_guard_pre.py (the read-side deny message), which is PR A / merge-guard: is_dangerous_command false-positive — gh issue comment body quoting a dangerous command is rejected (substring-scan brittleness) #1037 territory and which PR B must not touch. The tokenization fix lives in the shared extract_privileged_flags and/or the mint's flag_scan_text argument. Folding would collide with PR A's file and re-open the ratified, in-flight cluster plan (PR A already shipped as v4.4.45).
4. Lineage is security: --admin and other privileged flags ride past merge-guard auth binding #1042, not merge-guard mint ergonomics: silent no-mint on format mismatch + multi-prompt approval (UX hardening) #1052. This sharpens the bound_flags axis security: --admin and other privileged flags ride past merge-guard auth binding #1042 introduced.

Note an internal inconsistency this exposes: the existing deny message advises re-approving "with the literal command embedded" (L1150-1157), and the read-side comment says "NEVER suggest running the bare command or simplifying the question" (L1147-1149) — yet for THIS bug re-embedding the full literal re-introduces the backtick artifact and fails again; the only recovery is to simplify (drop the flags). That reinforces that the real fix is mint-side tokenization, not message wording.

Fix direction (NEVER weaken the #1042 set-equality bind)

1. Primary — make the mint scan the SAME undelimited input the read side scans. Either pass the located command region (not the full option prose) as flag_scan_text in _mint_context_from_bundle, or strip the wrapping markdown-backtick/quote delimiters before/within extract_privileged_flags so a trailing delimiter never leaks into a value-carrying flag's value. Result: mint bound_flags set-equals read bound_flags for a legitimate, truly-identical command → it authorizes.
2. Secondary (read-side legibility) — name the mismatching axis. When bound_flags is the sole divergence, the deny message should say e.g. "the approval bound flags {X} but the command carries {Y}" instead of the generic "approved operation does not match" (the bound_flags axis is the subtlest: op-type + PR-number look identical to the operator while a --repo value silently differs). This edit is in merge_guard_pre.py and must land in a separate change from PR B (PR A's file).

Acceptance criteria

• Mint and read tokenize identical input for privileged-flag extraction: extract_privileged_flags("--repo X", "merge") == extract_privileged_flags("--repo X", "merge")(no delimiter leaks into the value). Cover the-Rshort form,--match-head-commit, and a trailing boolean --admin` (currently silently dropped from the mint set).
• End-to-end: a single AskUserQuestion approval embedding the literal gh pr merge <N> --squash --delete-branch --repo <owner/repo> mints a token whose bound_flags set-equals the executed command's, so _token_matches_command returns True (authorizes) without dropping the privileged flags.
• Never-escalate preserved: an explicit test asserts that an execution-time escalation (the real command carries an extra privileged flag not in the token) still REFUSES. The security: --admin and other privileged flags ride past merge-guard auth binding #1042 set-equality bind at merge_guard_pre.py L1074 is unchanged.
• Over-block-only confirmation: the fix only flips refuse→authorize for truly-identical commands; it never flips refuse→authorize for a command carrying MORE privilege than approved (no under-block introduced).
• (Legibility) When bound_flags is the sole mismatching axis, the read-side deny message names that axis specifically ("the approval bound flags {X} but the command carries {Y}"). This change is scoped to merge_guard_pre.py and is NOT part of PR B.

Cross-references

• security: --admin and other privileged flags ride past merge-guard auth binding #1042 — introduced the bound_flags never-escalate axis and the widened mint flag_scan_text scan; this is its direct follow-up.
• merge-guard mint ergonomics: silent no-mint on format mismatch + multi-prompt approval (UX hardening) #1052 — the mint-side no-mint ergonomics sibling (read-vs-mint mirror); keep this case OUT of PR B to avoid pushing merge_guard_pre.py edits into post.py-only work.
• merge_guard: decline-veto false-positives on command-literal tokens (--no-* flags, branch names) #1049 — its force-push:{--no-verify} decline-veto fix interacts with the same flag-binding path.
• merge-guard: is_dangerous_command false-positive — gh issue comment body quoting a dangerous command is rejected (substring-scan brittleness) #1037 / v4.4.45 — owns merge_guard_pre.py (PR A); the read-side legibility edit must coordinate with it, not PR B.

[michael-wojcik]

Resolved in #1070 (v4.4.48) — the mint backtick-strip itself shipped in v4.4.46; #1070 added the missing mint==read parity regression test (the agreed test-only disposition). Auto-close keyword fired only for #1043; closing manually.