justinmclean opened a new issue, #188: URL: https://github.com/apache/airflow-steward/issues/188
# Security-language PRs are not flagged for ASF disclosure-process review ## Summary Neither the triage skill nor the code-review skill checks whether a PR's title, body, or commit messages contain language that signals a security fix. Under the ASF vulnerability-handling process, this language should be neutralised (or the CVE process verified complete) before the PR goes any further. Currently a maintainer could triage and later approve such a PR without ever being prompted to check. ## ASF policy requirement `https://www.apache.org/security/committers.html`: > *"Messages associated with any commits should not make any reference to > the security nature of the commit."* The full process requires that nothing referencing the vulnerability appear in public-facing content — PR titles, bodies, commit messages — until the CVE status is `READY` and the public announcement has been sent. After announcement, the references are fine. ## Why both triage and code-review need to handle this **Triage** is the first skill to see a PR. Flagging security language here gives the maintainer (and contributor) the earliest possible opportunity to edit the title, body, or commit messages before the PR accumulates any further review activity. PR titles and bodies are editable at any point, and commit messages can be amended before merge, so early detection is directly actionable. **Code review** is a safety net. If a PR slipped through triage — or triage ran before the security language was added — the review skill should still surface the warning. It also needs to carry the note through to the review body so the contributor sees it, regardless of disposition. ## Current behaviour Neither skill scans for CVE IDs or security-nature keywords. A PR titled "Fix SQL injection in DagBag serializer" with commit message "Fix SQL injection vulnerability" passes through both skills unchecked. ## Expected behaviour ### Triage skill (`classify-and-act.md`) Add a new row to the decision table, positioned **before row 8** (the first row that takes a destructive action), so it fires on otherwise-clean PRs that would otherwise reach `passing`: | # | Precondition | Classification | Action | Reason template | |---|---|---|---|---| | new | PR title, body, or any commit message matches `security_language_signal` | `security_language_signal` | `comment` | Security-language detected — ask contributor to neutralise or confirm CVE process complete | The `security_language_signal` predicate matches (case-insensitive) any of: - CVE IDs: `CVE-\d{4}-\d+` - Phrases: "security vulnerability", "security issue", "security fix", "security bug", "security flaw", "security patch", "arbitrary code execution", "remote code execution", "RCE", "SQL injection", "XSS", "CSRF", "SSRF", "path traversal", "directory traversal", "privilege escalation", "auth bypass", "authentication bypass", "authorization bypass", "insecure deserialization", "heap overflow", "buffer overflow", "use-after-free", "exploit", "exploitable" The comment body (new template in `comment-templates.md`) should: 1. Quote the matched text and its location (title / body / commit SHA). 2. Cite the ASF policy sentence verbatim. 3. Ask the contributor to either: (a) edit title/body and amend commit messages to be neutral, or (b) confirm that the CVE is already publicly announced (link to the announcement). 4. Note that the PR will not advance to maintainer review until one of those two conditions is met. This row does **not** convert the PR to draft or close it — it leaves it open so the contributor can edit in place. ### Code-review skill (`review-flow.md`) Add a security-disclosure signal scan at the end of Step 3 (after the existing prompt-injection check). If `security_language_signal` matches, surface a pre-review warning that requires explicit maintainer acknowledgment before Step 4 begins. The warning text carries through to the final review body regardless of disposition. Full pattern list and warning template are the same as above. ## Affected files - `.claude/skills/pr-management-triage/classify-and-act.md` — new decision-table row + `security_language_signal` predicate definition - `.claude/skills/pr-management-triage/comment-templates.md` — new `security-language` comment template - `.claude/skills/pr-management-code-review/review-flow.md` — Step 3 signal scan (safety net) - `.claude/skills/pr-management-code-review/criteria.md` — Security model section updated to document both checks ## Out of scope Detecting actual security vulnerabilities in diff code is already handled by the "Security model" category in the code-review skill's criteria.md. No change to that logic is needed here. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
