From ed24990f5acb53609c7ec3415912c8d2c95f4958 Mon Sep 17 00:00:00 2001 From: Davide Polato Date: Tue, 27 Oct 2026 17:20:41 +0100 Subject: [PATCH] fix(security-issue-deduplicate): keep the CVE both trackers share When the kept and the dropped tracker carry the same CVE ID, classify returned that ID as `retract`, so the proposal offered item 7 as "retract CVE-X, merged into CVE-X": a retraction of the record the kept tracker still needs. classify now proposes a retraction only when the two IDs differ, and apply-plan refuses item 7 when the ID to retract is the kept CVE. The skill and the lifecycle spec say the same: a retraction needs two distinct IDs. Signed-off-by: Davide Polato Generated-by: Claude Code (Opus 5.5) --- .../issue_deduplicate/apply.py | 2 ++ .../issue_deduplicate/gate.py | 2 +- .../issue_deduplicate/apply.py | 2 ++ .../issue_deduplicate/gate.py | 2 +- .../tests/test_issue_deduplicate.py | 20 +++++++++++++++++++ .../specs/security-issue-lifecycle.md | 3 ++- 6 files changed, 28 insertions(+), 3 deletions(-) diff --git a/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/apply.py b/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/apply.py index 39ba26408..b41e1f44e 100644 --- a/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/apply.py +++ b/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/apply.py @@ -155,6 +155,8 @@ def ref(value: object) -> str: if 7 in chosen: if not retract or not cve_id: return {"ok": False, "refused": "item 7 retracts the dropped side's CVE and needs both CVE IDs"} + if retract == cve_id: + return {"ok": False, "refused": f"item 7 would retract {cve_id}, the kept tracker's CVE"} steps.append( { "item": 7, diff --git a/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/gate.py b/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/gate.py index fcba1ba41..fc48fc141 100644 --- a/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/gate.py +++ b/plugins/magpie-security-tools/tools/security-management/src/security_management/issue_deduplicate/gate.py @@ -207,7 +207,7 @@ def link(side: Mapping[str, Any]) -> str: f"tracker's {keep_cve} ({states['keep']}): keep the drop tracker instead.", } ) - else: + elif drop_cve != keep_cve: retract = drop_cve if probe and not found: return { diff --git a/tools/security-management/src/security_management/issue_deduplicate/apply.py b/tools/security-management/src/security_management/issue_deduplicate/apply.py index 39ba26408..b41e1f44e 100644 --- a/tools/security-management/src/security_management/issue_deduplicate/apply.py +++ b/tools/security-management/src/security_management/issue_deduplicate/apply.py @@ -155,6 +155,8 @@ def ref(value: object) -> str: if 7 in chosen: if not retract or not cve_id: return {"ok": False, "refused": "item 7 retracts the dropped side's CVE and needs both CVE IDs"} + if retract == cve_id: + return {"ok": False, "refused": f"item 7 would retract {cve_id}, the kept tracker's CVE"} steps.append( { "item": 7, diff --git a/tools/security-management/src/security_management/issue_deduplicate/gate.py b/tools/security-management/src/security_management/issue_deduplicate/gate.py index fcba1ba41..fc48fc141 100644 --- a/tools/security-management/src/security_management/issue_deduplicate/gate.py +++ b/tools/security-management/src/security_management/issue_deduplicate/gate.py @@ -207,7 +207,7 @@ def link(side: Mapping[str, Any]) -> str: f"tracker's {keep_cve} ({states['keep']}): keep the drop tracker instead.", } ) - else: + elif drop_cve != keep_cve: retract = drop_cve if probe and not found: return { diff --git a/tools/security-management/tests/test_issue_deduplicate.py b/tools/security-management/tests/test_issue_deduplicate.py index 8e9d33963..8fb913f6a 100644 --- a/tools/security-management/tests/test_issue_deduplicate.py +++ b/tools/security-management/tests/test_issue_deduplicate.py @@ -172,6 +172,13 @@ def test_case_4_already_duplicate(capsys, tmp_path): None, "CVE-2025-2222", ), + ( + side(42, ["airflow"], cve_link=C1, cve_state="allocated"), + side(67, ["airflow"], cve_link=C1, cve_state="allocated"), + "proceed", + None, + None, + ), ( side(42, ["airflow"], cve_link=C1, cve_state="allocated"), side(67, ["airflow"], cve_link=C2, cve_state="publish-ready"), @@ -602,6 +609,19 @@ def test_case_2_with_blockers(capsys, tmp_path): assert "**Still to resolve:**" in out["text"] +def test_apply_plan_refuses_to_retract_the_kept_cve(capsys, tmp_path): + data = { + "tracker": TRACKER, + "keep": {"number": 42}, + "drop": {"number": 67}, + "scratch": "/s", + "items": [7], + "cve_id": "CVE-2025-1111", + "retract_cve": "CVE-2025-1111", + } + assert run(capsys, tmp_path, "apply-plan", data)[0] == 2 + + def test_apply_plan_refuses_a_malformed_cve_id(capsys, tmp_path): base = { "tracker": TRACKER, diff --git a/tools/spec-loop/specs/security-issue-lifecycle.md b/tools/spec-loop/specs/security-issue-lifecycle.md index cfb7eb491..44c4b7f55 100644 --- a/tools/spec-loop/specs/security-issue-lifecycle.md +++ b/tools/spec-loop/specs/security-issue-lifecycle.md @@ -177,7 +177,8 @@ publication, with a human gate and an audit-log entry at every step. are open with the same single scope label and neither `duplicate`, applies the CVE rules (a CVE only on the dropped side means swap, a `public` record refuses, the kept CVE must be at least as far along and - the dropped one is retracted), reads both bodies' fields (the CVE JSON + the dropped one is retracted, unless both trackers carry the same CVE), + reads both bodies' fields (the CVE JSON attachment cut off), and rewrites the kept body field by field through github-body-field's `replace_field`, so the attachment and extra fields stay byte-exact: the dropped report verbatim in a collapsed *Second