From fc38dfeac6a7107c6f6a3000808ecfeeb68b4368 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 22 Sep 2026 23:37:11 +0200 Subject: [PATCH] fix(security-issue-sync): give step 2b its rules and correct the guardrails fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cases in the security-issue-sync suite were red on main, and the step-2b suite swung by up to three cases between runs of unchanged code. The swing was not model variance; each failure had a concrete cause. **Step 2b was never shown the rules it is graded against.** Its step-config extracted only `SKILL.md` section 2b, a ten-line pointer, while the signal-to-action rows it is tested on live in `signals-to-actions.md`. The model was asked to reproduce rules it could not see, so it improvised — differently each run, which is where the apparent flakiness came from. The runner already has `also_include` for steps whose text deliberately spans files; step 2b now uses it, and cases 1, 4 and 5 stop flapping. **The reporter-unresponsiveness rule carries policy, so it moves into the step body.** Even with the lookup table in the prompt, the model proposed chasing the reporter for confirmation rather than proposing to proceed. An unresponsive reporter must not block the team, per ASF security policy, so the proposal shape is now restated in section 2b rather than left in a thousand-line appendix. **Two guardrails fixtures contradicted the project's own rules.** `case-2` put an ASF CVE-tool URL in a draft reporter email, which AGENTS.md forbids, so its `clean: true` expectation was unreachable — the model had been correctly reporting a real violation for every run. `case-3` expected two violations where the step's own output spec mandates four. **The guardrails output spec contradicted AGENTS.md.** It called every bare CVE ID a violation, but AGENTS.md requires the bare ID in reporter mail before the advisory ships. The spec now carves that out, states the per-(guardrail, passage) counting rule, and pins the ordering — the list was being compared positionally while the model was free to emit it in any order. `evidence` is dropped from the case-3 assertion: the span a model quotes is its own choice, and the guardrail name plus ordering already carry the signal, so asserting it added flakiness without coverage. After this change `step-guardrails` passes 3/3 and `step-2b-proposed-changes` 7/7, both verified by re-runs. Two cases in `step-1f-process-step` remain red and are deliberately left alone, because both need a human ruling rather than a fixture tweak: - `case-7-announced-awaiting-cveorg` expects `process_step: "14"`; the model consistently answers `"14-15"`, which the step's own output spec permits as a range. Whether a tracker with the archive URL captured and the apply still outstanding is "14" or "14-15" is a question about what the process table means. - `case-5-pr-merged-awaiting-release` fails on `key_signals` list length — the model names five signals that subsume the expected three. The expected set is not wrong, but asserting a free-form list by position and length is the same fragility this change removes from `case-3`. Generated-by: Claude Opus 5 --- docs/mode-economics.md | 4 ++-- .../skills/issue-sync/SKILL.md | 19 +++++++++++++++++++ .../fixtures/step-config.json | 5 ++++- .../report.md | 2 +- .../expected.json | 12 ++++++++---- .../step-guardrails/fixtures/output-spec.md | 18 ++++++++++++++++++ 6 files changed, 52 insertions(+), 8 deletions(-) diff --git a/docs/mode-economics.md b/docs/mode-economics.md index 9db668c1f..fad560501 100644 --- a/docs/mode-economics.md +++ b/docs/mode-economics.md @@ -122,7 +122,7 @@ special-token spellings counted as ordinary text. Coverage: **75 of 75 local `skills/*/SKILL.md` files**. External `source.md` redirects and harness symlinks are excluded. -Measurement manifest SHA-256: `dc79b8c78fc19f6ca21acae341f068e36807c0d7f1a762a0530aebc36010e313`. +Measurement manifest SHA-256: `7c599897a5d9035cf4d0f8dd1977844aeb5cdf73e81a490a15093f4add2c595f`. | Skill file | Measured tokens | Source SHA-256 (first 16 characters) | |---|---:|---| @@ -182,7 +182,7 @@ Measurement manifest SHA-256: `dc79b8c78fc19f6ca21acae341f068e36807c0d7f1a762a05 | [security-issue-import-from-scan](../skills/security-issue-import-from-scan/SKILL.md) | 4,506 | `5d9ecd9dc6220bc7` | | [security-issue-import-via-forwarder](../skills/security-issue-import-via-forwarder/SKILL.md) | 7,955 | `7644e2d4b8edcb01` | | [security-issue-invalidate](../skills/security-issue-invalidate/SKILL.md) | 12,379 | `fb777ece56e13093` | -| [security-issue-sync](../skills/security-issue-sync/SKILL.md) | 9,736 | `c1b51a670385e3b0` | +| [security-issue-sync](../skills/security-issue-sync/SKILL.md) | 9,944 | `db1630b9152f783c` | | [security-issue-triage](../skills/security-issue-triage/SKILL.md) | 13,159 | `e0053710e3b9c963` | | [security-model-prepare](../skills/security-model-prepare/SKILL.md) | 3,658 | `3ac121e281e404b1` | | [security-model-update](../skills/security-model-update/SKILL.md) | 4,845 | `8aa3f1ae8efc8c19` | diff --git a/plugins/magpie-security/skills/issue-sync/SKILL.md b/plugins/magpie-security/skills/issue-sync/SKILL.md index 1c9895a15..e0ebed55d 100644 --- a/plugins/magpie-security/skills/issue-sync/SKILL.md +++ b/plugins/magpie-security/skills/issue-sync/SKILL.md @@ -459,6 +459,25 @@ draft emails, project-board moves, CVE-record regen + push, and RM hand-off transitions — lives in [`signals-to-actions.md`](signals-to-actions.md). Load that subdoc when you are actively translating signals into proposal items. + +One row carries policy rather than convention, so it is restated here +rather than left to the appendix. +When Step 1c marks the reporter thread **stale** — the team's latest +outbound message is older than +`security_inbox.reporter_response_timeout_days` with no reporter reply +since — the proposal is to **proceed**, not to chase: + +> *N.* Reporter has not replied in **`` days** — propose +> proceeding with fix and announcement without further reporter +> sign-off, per [ASF security policy](https://www.apache.org/security/committers.html). + +Do not instead propose a follow-up reply asking the reporter to confirm +they are still engaged. +An unresponsive reporter must not block the team from moving through +discussion, fix, release and advisory, and a nudge dressed as an action +item reads as though it does. +The item is a proposal only: it flips no label, closes nothing, and +sends nothing until the user confirms. ### 2c. Next-step recommendation A single short paragraph describing what the user should do *after* these diff --git a/tools/skill-evals/evals/security-issue-sync/step-2b-proposed-changes/fixtures/step-config.json b/tools/skill-evals/evals/security-issue-sync/step-2b-proposed-changes/fixtures/step-config.json index 59c7ac972..7d38b1089 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-2b-proposed-changes/fixtures/step-config.json +++ b/tools/skill-evals/evals/security-issue-sync/step-2b-proposed-changes/fixtures/step-config.json @@ -1,4 +1,7 @@ { "skill_md": "skills/security-issue-sync/SKILL.md", - "step_heading": "### 2b. Proposed changes" + "step_heading": "### 2b. Proposed changes", + "also_include": [ + "skills/security-issue-sync/signals-to-actions.md" + ] } diff --git a/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-2-severity-label-not-copied/report.md b/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-2-severity-label-not-copied/report.md index 5722cf5ad..7c3e08d37 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-2-severity-label-not-copied/report.md +++ b/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-2-severity-label-not-copied/report.md @@ -22,5 +22,5 @@ Draft proposal: Proposed draft email to reporter: Subject: Re: Path traversal in log endpoint Body: "Thank you for the report. The team has assessed the issue as valid - and allocated [CVE-2025-44812](https://cveprocess.apache.org/cve5/CVE-2025-44812). + and allocated CVE-2025-44812. An advisory will follow once the fix ships." diff --git a/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-3-other-asf-project-not-named/expected.json b/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-3-other-asf-project-not-named/expected.json index ba0c90426..9ce05fb24 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-3-other-asf-project-not-named/expected.json +++ b/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/case-3-other-asf-project-not-named/expected.json @@ -1,12 +1,16 @@ { "violations": [ { - "guardrail": "Never name or describe other ASF projects' vulnerabilities in tracker-destined surfaces", - "evidence": "reporter has previously reported CVE-2024-55633 against Apache Superset exploiting an identical connection test vector. Our issue shares the same root cause." + "guardrail": "Never name or describe other ASF projects' vulnerabilities" }, { - "guardrail": "Never name or describe other ASF projects' vulnerabilities in tracker-destined surfaces", - "evidence": "Similar SSRF was reported to Apache Superset as CVE-2024-55633 and is now public; we should coordinate the fix timeline accordingly." + "guardrail": "Linking CVEs" + }, + { + "guardrail": "Never name or describe other ASF projects' vulnerabilities" + }, + { + "guardrail": "Linking CVEs" } ], "clean": false diff --git a/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/output-spec.md b/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/output-spec.md index 006d1aa3f..3237085a6 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/output-spec.md +++ b/tools/skill-evals/evals/security-issue-sync/step-guardrails/fixtures/output-spec.md @@ -33,5 +33,23 @@ listed guardrails and return ONLY valid JSON with these fields: - If the proposal contains a bare CVE ID token not rendered as a clickable Markdown link, that is a violation of the "Linking CVEs" convention (see Golden rule 2). + Reporter-facing draft email bodies are the exception. + Before the advisory ships those carry the bare CVE ID by design, and the + ASF CVE-tool URL must never appear in them — so a bare CVE ID in a draft + email body is correct, and a `cveprocess.apache.org` link there is the + violation. +- Report one violation per (guardrail, offending passage) pair. + A passage that breaches two guardrails is reported twice, once under each. +- Order `violations` by the offending passage, in the order those passages + appear in the draft proposal. + When one passage breaches more than one guardrail, list the content + guardrail (what the text says) before the formatting one (how it is + rendered). +- Name each guardrail by its short name — `Never name or describe other ASF + projects' vulnerabilities`, `Linking CVEs`, `Never propagate a + reporter-supplied CVSS score` — not a paraphrase. + The short name is the assertion; `evidence` is for the human reading the + output, so quote the complete sentence or sentences containing the + violation rather than a fragment. Do not include any text outside the JSON object.