justinmclean opened a new pull request, #189: URL: https://github.com/apache/airflow-steward/pull/189
## Problem Neither the triage skill nor the code-review skill checked whether a PR's title, body, or commit messages contained language indicating a security fix. Under the ASF vulnerability-handling process, such references must not appear in public-facing content until the CVE is formally announced: > *"Messages associated with any commits should not make any reference to > the security nature of the commit."* > — https://www.apache.org/security/committers.html A maintainer could previously triage and approve a PR like: ``` Title: Fix path traversal vulnerability in log viewer endpoint Body: This fixes a security issue. CVE-2024-47554 is pending announcement. Commit: "Fix path traversal vulnerability in log viewer" ``` …without ever being prompted to verify that the CVE process was complete. PR titles, bodies, and commit messages are all editable before merge, so early detection is directly actionable. ## Changes **`.claude/skills/pr-management-triage/classify-and-act.md`** *(primary fix)* - Row 7 renamed to **7a**; new row **7b** added immediately after. - Row 7b classification: `security_language_signal` / Action: `comment`. Fires after the "too fresh" skip (7a) and before the first destructive action (row 8), so it catches otherwise-clean PRs that would otherwise reach `passing`. - New `security_language_signal` predicate in the glossary — case-insensitive scan of `title`, `body`, and all `commits.nodes[].message` for CVE IDs (`CVE-\d{4}-\d+`) and a specific list of security-nature phrases. Each match is recorded with its location (title / body / commit SHA) for use in the comment template. **`.claude/skills/pr-management-triage/comment-templates.md`** *(primary fix)* - New `security-language` template used by row 7b's `comment` action. - Quotes each matched text and location verbatim. - Quotes the ASF policy sentence. - Gives the contributor two explicit paths forward: (a) neutralise the language in title, body, and commit messages, or (b) confirm the CVE is already publicly announced with a link. - Includes the standard AI-attribution footer. **`.claude/skills/pr-management-code-review/review-flow.md`** *(safety net)* - New `### Security-disclosure signal scan` subsection at the end of Step 3. - Same pattern list as the triage predicate — scans title, body, and all commit messages before examining the diff. - On any match: surfaces a pre-review warning, quotes the matched text and location, requires explicit `[Y]es` acknowledgment before Step 4 begins, and carries the warning into the final review body regardless of disposition. **`.claude/skills/pr-management-code-review/criteria.md`** *(safety net)* - Short blockquote pointer added at the top of the "Security model — calibration" section, cross-referencing the Step 3 disclosure scan. Existing calibration text is unchanged. ## Testing **Structural validation** `tools/skill-validator` run against all SKILL.md files post-change. Result: 0 violations in pr-management skills. **Functional dry-run — triage skill** *Positive case* — PR #99998, title `"Fix path traversal vulnerability in log viewer endpoint"`, body contains `"security issue"` and `"CVE-2024-47554"`, commit `"Fix path traversal vulnerability in log viewer"`: - Pre-filters pass through (external contributor, not draft, no prior triage marker, not own PR). - Row 7a: created 2h ago, not `< 30min` → no match. - Row 7b: 4 matches across title, body, and commit message → fires. - Action: `comment` posted using `security-language` template, listing all matched locations verbatim. PR stays open; does not advance to `mark-ready`. ✓ *Negative case* — PR #99997, title `"Improve scheduler heartbeat retry logic"`, neutral body and commits: - Row 7b: 0 matches → skip. - Row 20: green CI, no conflicts, no threads → `mark-ready`. No security comment posted. ✓ **Functional dry-run — code-review skill** *Positive case* — PR with title `"Fix SQL injection vulnerability in DagBag serializer"`, body referencing `"security issue"`, commit `"Fix SQL injection in DagBag"`: 3 matches across all three locations; pre-review warning fires before Step 4; explicit acknowledgment required. ✓ *Negative case* — PR `"Fix N+1 query in DagBag.get_dag()"`, neutral body and commits: 0 matches; Step 4 proceeds immediately without pause. ✓ -- 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]
