diff --git a/skills/pr-management-triage/SKILL.md b/skills/pr-management-triage/SKILL.md index a98bb25d2..cfeb391c0 100644 --- a/skills/pr-management-triage/SKILL.md +++ b/skills/pr-management-triage/SKILL.md @@ -442,6 +442,13 @@ notified: blocks every other. See [`tools/agent-guard`](../../tools/agent-guard/README.md) and [`comment-templates.md`](comment-templates.md#the-folded-maintainer-triage-note--the-single-contributor-channel). +- Exemption — **your own PR/issue**: this rule targets triaging + *other* people's PRs. When the operator is themselves the author + (author == the authenticated `gh` user), the guard allows + `@`-mentioning maintainers/reviewers — nudging your own reviewers + from your own PR is a legitimate, deliberate act. A one-off + `MAGPIE_ALLOW_MENTIONS=1` override is the escape hatch for any + other intentional exception. This supersedes Golden rule 9's "pings still notify a maintainer" expectation for the operator/reviewer side: F5a/F5b still make the diff --git a/skills/pr-management-triage/guards/mention.py b/skills/pr-management-triage/guards/mention.py index 8902a3c58..14c003b43 100644 --- a/skills/pr-management-triage/guards/mention.py +++ b/skills/pr-management-triage/guards/mention.py @@ -23,7 +23,13 @@ author-directed comments (`gh pr/issue comment`) — in both, the author's @-mention is permitted (it is the intended "your move" signal) and any other @-mention (a maintainer: operator, reviewer, CODEOWNER, team) is blocked. -Maintainer handles must be backtick-quoted so they never notify. Discovered by +Two exceptions lift the block: an explicit ``MAGPIE_ALLOW_MENTIONS=1`` override +(a deliberate one-off, requested by the operator), and the operator commenting +on their **own** PR/issue — when the target's author is the authenticated ``gh`` +user, mentioning maintainers is a legitimate self-directed nudge to one's own +reviewers, not the drive-by maintainer spam this guard exists to stop. +Otherwise maintainer handles must be backtick-quoted so they never notify. +Discovered by the agent-guard PreToolUse dispatcher from a guards.d directory — see tools/agent-guard for the engine and the GuardContext API. Import-free: everything comes from ``ctx``. @@ -70,6 +76,13 @@ def guard(ctx): "author is known, drop the @-mentions (use backticked `login`), or override " "with MAGPIE_ALLOW_MENTIONS=1 if the mention is intentional." ) + # Operator's own PR/issue: when the target's author is the authenticated gh + # user, mentioning maintainers is a self-directed nudge to one's own reviewers + # (legitimate), not the drive-by maintainer spam this guard blocks. Resolution + # failing falls through to the normal author-only rule (safe default). + operator = ctx.run(["gh", "api", "user", "--jq", ".login"]) + if operator and operator.lower() == author.lower(): + return None offenders = sorted({m for m in mentions if m != author.lower()}) if offenders: return ( diff --git a/tools/agent-guard/README.md b/tools/agent-guard/README.md index b237aa688..90a74e479 100644 --- a/tools/agent-guard/README.md +++ b/tools/agent-guard/README.md @@ -64,7 +64,7 @@ way — see [Contributing guards](#contributing-guards)): | Guard | Owner skill | Blocks | Rule it enforces | |---|---|---|---| -| `mention` | `pr-management-triage` | `gh pr comment` / `gh issue comment` that `@`-mentions anyone other than the PR/issue author; **any** `@`-mention in `gh pr edit --body[-file]` | denoise: author-directed feedback never pings maintainers; body edits stay silent | +| `mention` | `pr-management-triage` | `gh pr comment` / `gh issue comment` that `@`-mentions anyone other than the PR/issue author; **any** `@`-mention in `gh pr edit --body[-file]` | denoise: author-directed feedback never pings maintainers; body edits stay silent. Exempt: the operator commenting on their **own** PR/issue (author == authenticated `gh` user), and the `MAGPIE_ALLOW_MENTIONS=1` override | | `mark-ready` | `pr-management-triage` | adding `ready for maintainer review` while the PR head SHA has GitHub Actions runs awaiting approval | Golden rule 1b | | `security-language` | `security-issue-fix` | a CVE id / security-fix language in a **public** `gh pr create`/`gh pr edit` title/body (not comments) | public-PR scrubbing | diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index 8693d44ab..ee918ac0a 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -51,6 +51,19 @@ def _stub(args, cwd=None): return _stub +def gh_stub(*, author, operator="operator-bot"): + """Distinguish the guard's two lookups: ``gh api user`` (operator identity) + vs. ``gh pr/issue view`` (target author). Default operator differs from the + author so a bare stub keeps the author-only rule in force.""" + + def handler(args): + if "api" in args and "user" in args: + return operator + return author + + return handler + + def dispatch(command): return agent_guard.dispatch(command, cwd=None) @@ -65,17 +78,39 @@ def test_guard_files_exist(): def test_mention_author_allowed(monkeypatch): - monkeypatch.setattr(agent_guard, "_run", fake_run(lambda a: "alice")) + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice"))) assert dispatch('gh pr comment 5 --body "@alice thanks"') is None def test_mention_non_author_denied(monkeypatch): - monkeypatch.setattr(agent_guard, "_run", fake_run(lambda a: "alice")) + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice"))) reason = dispatch('gh pr comment 5 --body "@bob please review"') assert reason and "bob" in reason and "mention" in reason -def test_fold_any_mention_denied(): +def test_mention_own_pr_allows_maintainers(monkeypatch): + # Operator commenting on their own PR may @-mention maintainers/reviewers. + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice", operator="alice"))) + assert dispatch('gh pr comment 5 --body "@bob @carol please take a look"') is None + + +def test_mention_own_pr_case_insensitive(monkeypatch): + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="Alice", operator="alice"))) + assert dispatch('gh pr comment 5 --body "@bob review please"') is None + + +def test_mention_others_pr_still_denied_when_operator_known(monkeypatch): + # Author is someone else; operator resolving successfully must not relax the rule. + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice", operator="carol"))) + reason = dispatch('gh pr comment 5 --body "@bob ping"') + assert reason and "bob" in reason and "mention" in reason + + +def test_fold_any_mention_denied(monkeypatch): + # Hermetic: a non-author maintainer @-mention in a fold edit is denied. + # Stubbed so it never depends on a real `gh pr view` (and so the own-PR + # exemption, author == operator, is not accidentally triggered). + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="bob", operator="carol"))) reason = dispatch('gh pr edit 5 --body "@alice heads up"') assert reason and "fold" in reason @@ -85,7 +120,7 @@ def test_fold_clean_allowed(): def test_mention_body_file(monkeypatch, tmp_path): - monkeypatch.setattr(agent_guard, "_run", fake_run(lambda a: "alice")) + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice"))) body = tmp_path / "b.md" body.write_text("@bob please look", encoding="utf-8") assert dispatch(f"gh pr comment 5 --body-file {body}") is not None @@ -98,7 +133,7 @@ def test_mention_author_unresolved_fails_closed(monkeypatch): def test_mention_override(monkeypatch): - monkeypatch.setattr(agent_guard, "_run", fake_run(lambda a: "alice")) + monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice"))) assert dispatch('MAGPIE_ALLOW_MENTIONS=1 gh pr comment 5 --body "@bob ping"') is None diff --git a/tools/cve-tool-vulnogram/record.md b/tools/cve-tool-vulnogram/record.md index 6aaa45722..2cea7d7f7 100644 --- a/tools/cve-tool-vulnogram/record.md +++ b/tools/cve-tool-vulnogram/record.md @@ -273,13 +273,14 @@ flow described in [*`#source` paste flow*](#source-paste-flow): corresponding body field on the tracker, wait for the JSON to regenerate, re-paste in `#source`, and re-preview. -5. **Send the advisory emails.** Vulnogram dispatches to - `` and ``. - **Then manually send a copy to +5. **Send the advisory emails.** The **Send these Emails** button + on Vulnogram's **OSS/ASF Emails** tab dispatches, in one action, + to ``, ``, and [`oss-security@lists.openwall.com`](https://oss-security.openwall.org/) - (required per the - [ASF security-committers policy](https://www.apache.org/security/committers.html)).** - On the tracker, add the + — the `oss-security` copy (required per the + [ASF security-committers policy](https://www.apache.org/security/committers.html)) + is sent automatically alongside the ASF lists, so there is no + separate manual send. On the tracker, add the `announced - emails sent` label and remove `fix released`. 6. **Wait for the publication-ready notification comment.** The diff --git a/tools/cve-tool-vulnogram/release-manager-handoff-comment-oauth-pushed.md b/tools/cve-tool-vulnogram/release-manager-handoff-comment-oauth-pushed.md index 13a5950d6..eb0820ebf 100644 --- a/tools/cve-tool-vulnogram/release-manager-handoff-comment-oauth-pushed.md +++ b/tools/cve-tool-vulnogram/release-manager-handoff-comment-oauth-pushed.md @@ -123,7 +123,7 @@ With the record in `READY`, click the [OSS/ASF Emails tab](EMAIL_TAB_URL) (the ` **If anything looks wrong**: don't edit it in Vulnogram. Comment on this tracker (just `@potiuk: the X field needs Y`) and we'll fix the corresponding body field here, regenerate the JSON, and re-push within the next sync. Re-preview after that. -**If everything looks right**: click the **Send these Emails** button on the OSS/ASF Emails tab. The advisory ships to `USERS_LIST` and `ANNOUNCE_LIST`. **Then manually send a copy to [`oss-security@lists.openwall.com`](https://oss-security.openwall.org/) (required per the [ASF security-committers policy](https://www.apache.org/security/committers.html)).** **That is the only Vulnogram send action you make for this CVE.** +**If everything looks right**: click the **Send these Emails** button on the OSS/ASF Emails tab. That one action ships the advisory to `USERS_LIST`, `ANNOUNCE_LIST`, and [`oss-security@lists.openwall.com`](https://oss-security.openwall.org/) — the tab is the "**OSS**/ASF Emails" tab precisely because it dispatches the `oss-security` copy (required per the [ASF security-committers policy](https://www.apache.org/security/committers.html)) automatically alongside the ASF lists. **You do not send a separate copy by hand — that is the only send action you make for this CVE.** > ⚠️ **Do not touch the tracker labels yourself.** Sync flips `fix released` → `announced - emails sent` + `announced` automatically when it sees the advisory in the public archive (usually within the same day). If you flip them manually you race the automation. diff --git a/tools/cve-tool-vulnogram/release-manager-handoff-comment.md b/tools/cve-tool-vulnogram/release-manager-handoff-comment.md index 19c19f3ce..9ae7d3494 100644 --- a/tools/cve-tool-vulnogram/release-manager-handoff-comment.md +++ b/tools/cve-tool-vulnogram/release-manager-handoff-comment.md @@ -125,7 +125,7 @@ With the record in `READY`, click the [OSS/ASF Emails tab](EMAIL_TAB_URL) (the ` **If anything looks wrong**: don't edit it in Vulnogram. Comment on this tracker (just `@potiuk: the X field needs Y`) and we'll fix the corresponding body field here, regenerate the JSON, and re-push within the next sync. Re-preview after that. -**If everything looks right**: click the **Send these Emails** button on the OSS/ASF Emails tab. The advisory ships to `USERS_LIST` and `ANNOUNCE_LIST`. **Then manually send a copy to [`oss-security@lists.openwall.com`](https://oss-security.openwall.org/) (required per the [ASF security-committers policy](https://www.apache.org/security/committers.html)).** **That is the only Vulnogram send action you make for this CVE.** +**If everything looks right**: click the **Send these Emails** button on the OSS/ASF Emails tab. That one action ships the advisory to `USERS_LIST`, `ANNOUNCE_LIST`, and [`oss-security@lists.openwall.com`](https://oss-security.openwall.org/) — the tab is the "**OSS**/ASF Emails" tab precisely because it dispatches the `oss-security` copy (required per the [ASF security-committers policy](https://www.apache.org/security/committers.html)) automatically alongside the ASF lists. **You do not send a separate copy by hand — that is the only send action you make for this CVE.** > ⚠️ **Do not touch the tracker labels yourself.** Sync flips `fix released` → `announced - emails sent` + `announced` automatically when it sees the advisory in the public archive (usually within the same day). If you flip them manually you race the automation.