[I] fix(pr-triage): the mark-ready guard can be bypassed with a repeated or comma-separated --add-label [magpie]

["dpol1 (via GitHub)" <gi...@apache.org>]

dpol1 opened a new issue, #1493:
URL: https://github.com/apache/magpie/issues/1493

   ### What
   
   `plugins/magpie-pr-management/skills/pr-triage/guards/mark_ready.py` enforces Golden rule 1b: never add the "ready for maintainer review" label while the PR head still has GitHub Actions runs awaiting approval. It decides whether a `gh pr edit` adds that label with:
   
   ```python
   label = ctx.opt("", "--add-label")
   ```
   
   `ctx.opt()` is `_opt_value()` (`tools/agent-guard/src/agent_guard/__init__.py:204-212`), which returns the value of the first matching flag and stops. `gh pr edit` accepts `--add-label` more than once, and each value can be a comma-separated list, so two forms add the ready label without the guard looking at it. Using the same fake `gh` as `test_mark_ready_pending_denied` (two runs awaiting approval), through `dispatch()`:
   
   ```text
   DENY   gh pr edit 5 --repo o/r --add-label "ready for maintainer review"
   ALLOW  gh pr edit 5 --repo o/r --add-label triaged --add-label "ready for maintainer review"
   ALLOW  gh pr edit 5 --repo o/r --add-label "triaged,ready for maintainer review"
   ```
   
   ### Why it matters
   
   The guard is the deterministic check behind rule 1b. In the two forms above it does not see the ready label, so the rule is left to the skill's instructions alone. The guard's fail-open paths (no PR number, a failed `gh` lookup) are deliberate; this one is not.
   
   #1246 fixed the neighbouring class, flag values mistaken for positionals. It did not cover repeated or comma-separated values.
   
   ### Suggested fix
   
   1. Add a multi-value accessor next to `opt()` on the guard context that returns every value of a flag, in both `--flag value` and `--flag=value` form.
   2. In `mark_ready.py`, split each `--add-label` value on commas and deny when any of them equals the ready label (trimmed and case-insensitive, as today).
   3. Add the two ALLOW cases above to `tools/agent-guard/tests/test_skill_guards.py` as DENY cases.
   
   ### Found via
   
   An architecture pass over how agent-guard parses `gh` argv; reproduced through `dispatch()` on `main` (`37670575`).
   
   ### Related
   
   - #1246: fix(agent-guard): consume gh flag values so guards cannot be bypassed by flag order.
   - `command_kinds()` (`__init__.py:686-687`) still classifies a `gh` call by `argv[1]`, so `gh -R o/r pr edit ...` is kind `gh:-R`. No shipped guard triggers on a `gh:<group>` kind today (they all declare `["gh"]`), so this is latent. `git` already goes through `git_subcommand_index()`; `gh_subcommand()` could do the same job for `gh`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] fix(pr-triage): the mark-ready guard can be bypassed with a repeated or comma-separated --add-label [magpie]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk closed issue #1493: fix(pr-triage): the mark-ready guard can be bypassed with a repeated or comma-separated --add-label
URL: https://github.com/apache/magpie/issues/1493


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org