Repository navigation
refactor(security): keep ASF defaults out of non-ASF drafts - #1905
Conversation
github-actions
Bot
added
family:security
Kaap10
left a comment
Kaap10
left a comment
There was a problem hiding this comment.
Thanks @Anurag-S1ngh for addressing this. The PR cleanly solves #1802 and makes the CVE tool URL and project naming checks vendor-neutral.
The scoping of ASF_PROJECTS to organization: ASF removes the false positive leak warnings for non-ASF adopters, and reading [meta].cve_tool_url keeps the generated attachment link, paste hint, and reference filtering consistent with the configured portal.
Both tools/ and plugins/magpie-security-tools/tools/ mirror trees are in sync, and local test runs pass cleanly across both test suites.
Left a small non-blocking inline note on URL parsing when a scheme is omitted.
potiuk
left a comment
potiuk
left a comment
There was a problem hiding this comment.
Making the cross-project name check and the CVE-tool URL follow the adopter's organization is the right direction, and the tests and plugin mirrors are thorough. Two points came up in review, both small enough that I pushed a fixup commit (0baf35b) to your branch rather than send it back:
- The public CVE record is not a private tool. The default
independentorganization setscve_authority.cve_tool_url: https://www.cve.org(organizations/independent/organization.md:72). With the new wiring,issue_sync checkandcheck-draftflagged thehttps://www.cve.org/CVERecord?id=…link that the editorial rules allow once a CVE is published, andbuild_referenceswould have dropped it fromreferences[].cve.org/www.cve.orgnow have no private URL to guard:shared/drafting.py(PUBLIC_CVE_HOSTS,cve_tool_url_patternreturnsNone),issue_sync/render.py(_cve_tool_pattern), andgenerate_cve_json/cve_json.py(_cve_tool_host), with tests in all three suites. - The ASF-name guardrail fails closed. An unset
organizationnow counts asASFinshared/drafting.pyandissue_sync/render.py, so a dropped key in the agent-built input no longer turns the check off on the default path. A test for each.
The plugin copies are mirrored. Please pull before pushing again.
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 guide.
Why: the draft and recap guardrails always compared text against the ASF project list, so a non-ASF adopter was flagged for naming a project that has nothing to do with its organisation. Read organization from the project config and only run the ASF comparison for organization: ASF.
Why: the generator linked every CVE to cveprocess.apache.org and treated that host as the one private reference to drop, so a non-ASF adopter got ASF links and its own tool URL leaked into references. Read [meta].cve_tool_url (or --cve-tool-url), keep the ASF URL as the default, and use the configured host for the link, the source hint and the filter. The draft guardrail builds its CVE-tool pattern from the same value.
Why: the new organization and cve_tool_url inputs change what the guardrails flag and which host the generated record links to, so the tool READMEs need to name them. Keep the ASF URL as the documented default and show the non-ASF override.
Why: a bare host (`cve.example.org`) leaves `urlparse`'s `netloc` empty, so the draft scan and the sync check fell back to the ASF host and the generator filtered nothing. The adopter's own tool URL then passed the guardrail and stayed in `references[]`, and the attachment link came out scheme-less and unclickable. Fill in `https://` when the scheme is omitted and match on `hostname` so a configured port still matches a plain URL. One helper now feeds the draft scan, the sync check and the generator, and the generator normalises the value once at config load and on `--cve-tool-url`. Generated-by: OpenCode (DeepSeek V4.1 Flash)
…ation The `independent` organization's CVE tool URL is https://www.cve.org, the public record. Treating it as a private tool flagged the cve.org link the editorial rules allow once a CVE is published, in both check-draft and issue-sync check, and would have filtered it out of references[]. The public record host now has no private URL to guard. An unset organization now counts as ASF, so dropping the key from the check input no longer turns off the other-ASF-project-name guardrail. Generated-by: Claude Opus 5
0baf35b to
3dea6be
Compare
|
Thanks for the fixes. I pulled the commit to my branch |
Summary
organization: ASF, so a non-ASF adopter is no longer flagged for naming a project from its own organisation.[meta].cve_tool_url(or--cve-tool-url), defaulting tohttps://cveprocess.apache.org, for the attachment link, the paste hint, and the host filtered out ofreferences[]. The draft guardrail uses the same host.Fixes #1802
Test plan
pytestintools/security-management: 714 passedpytestintools/cve-tool-vulnogram/generate-cve-json: 449 passedprek run --all-files --skip workspace-pytest: all hooks pass