Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions skills/pr-management-triage/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 14 additions & 1 deletion skills/pr-management-triage/guards/mention.py
Original file line number Diff line number Diff line change
Expand Up @@ -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``.
Expand Down Expand Up @@ -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 (
Expand Down
2 changes: 1 addition & 1 deletion tools/agent-guard/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |

Expand Down
45 changes: 40 additions & 5 deletions tools/agent-guard/tests/test_skill_guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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

Expand All @@ -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
Expand All @@ -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


Expand Down
13 changes: 7 additions & 6 deletions tools/cve-tool-vulnogram/record.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
`<users-list>` and `<announce-list>`.
**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 `<users-list>`, `<announce-list>`, 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down