fix(security): remove ASF address from skills - #1055
Conversation
Signed-off-by: Polly Labs <pollychen.lab@gmail.com>
potiuk
left a comment
There was a problem hiding this comment.
LGTM — and the security-cve-allocate half is a better fix than "delete the hardcoded address" usually gets.
What I checked
Both occurrences really were there, and this removes both:
skills/security-cve-allocate/SKILL.md:467 an email to `security@apache.org` with subject
skills/security-issue-sync/github-advisory.md:129 (`<security-team-list>`; for ASF, `security@apache.org`) with
Those are the only two in skills/ on main, so the stated grep -rn "security@apache.org" skills/ result holds after this change.
The substitution is the good part. The old fallback did not just name an ASF address, it encoded an ASF procedure — mail the ASF Security Team, they allocate and mail the ID back. Swapping that for "follow the fallback path documented by the active CVE-tool adapter" moves the ASF-specific mechanics behind the adapter boundary, which is where AGENTS.md § Placeholder convention wants them — "skills must stay project-agnostic so swapping projects is a config change, not a code change." A non-ASF adopter reading the old text had no path at all; now they are pointed at whichever adapter they actually run.
I verified the citation resolves rather than trusting lychee: tools/cve-tool-vulnogram/allocation.md exists and carries ## PMC-gated access at line 44.
Smaller observation
Worth a follow-up, not a change here.
github-advisory.md now reads (<security-team-list>) with …, where before the parenthetical glossed it as "for ASF, security@apache.org". Removing the gloss is right — that was the hardcoding. But it does mean the placeholder now carries the whole meaning on its own, and it is a one-off:
| placeholder | files using it | registered in AGENTS.md? |
|---|---|---|
<security-list> |
13 | yes — line 363, sourced from <project-config>/project.md → mailing_lists.security |
<security-team-list> |
1 (this file) | no |
<security-team-list> predates this PR, so nothing here invents it, and check-placeholders will not catch it — that hook scans for forbidden literals, not for unregistered placeholder names, which is why CI is green either way.
Normalising it to <security-list> would be a one-word change and would make the line resolvable by the same config key as the other thirteen. I would not fold it in here, since it is a pre-existing inconsistency in a file this PR is only touching in passing.
(Separately, and definitely not this PR's problem: projects/_template/project.md declares no mailing_lists block at all, so even the registered <security-list> has no key an adopter would fill in. That gap is worth its own issue.)
This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Apache Magpie handles maintainer review:
CONTRIBUTING.md § Opening a pull request.
|
@potiuk thanks |
Summary
security-cve-allocateand point fallback guidance at the active CVE-tool adapter instead.<security-team-list>placeholder.Fixes #998
Validation
grep -rn "security@apache.org" skills/returns no matchesuv run --project tools/skill-and-tool-validator --group dev skill-and-tool-validateuv run prek run --all-files