fix(tests): fix veto bug preventing disputed state, add manager notification and dispute-resolution routing

This commit is contained in:
kitos
2026-07-06 12:40:05 +02:00
parent 388c9773ab
commit f53e124c50
7 changed files with 448 additions and 8 deletions
+61 -4
View File
@@ -83,7 +83,10 @@ VALID_TRANSITIONS: dict[TestState, list[TestState]] = {
TestState.blue_evaluating: [TestState.blue_review], TestState.blue_evaluating: [TestState.blue_review],
TestState.blue_review: [TestState.in_review, TestState.blue_evaluating], TestState.blue_review: [TestState.in_review, TestState.blue_evaluating],
TestState.in_review: [TestState.validated, TestState.rejected, TestState.disputed], TestState.in_review: [TestState.validated, TestState.rejected, TestState.disputed],
TestState.disputed: [TestState.validated, TestState.rejected], TestState.disputed: [
TestState.validated, TestState.rejected,
TestState.red_executing, TestState.blue_evaluating,
],
TestState.rejected: [TestState.draft], TestState.rejected: [TestState.draft],
TestState.validated: [], TestState.validated: [],
} }
@@ -627,6 +630,44 @@ class TestEntity:
# Call self._events.append() # Call self._events.append()
self._events.append(DomainEvent("test_reopened")) self._events.append(DomainEvent("test_reopened"))
def resolve_dispute_reject(self, target: str) -> None:
"""Resolve a ``disputed`` test by routing rework to the team at fault.
Called when the lead who originally approved flips their vote to
agree with the rejection. Rather than the generic terminal
``rejected`` state (which forces a full draft restart for both
teams), the flipping lead identifies WHICH side's work needs
redoing — the test goes straight back to that team's active queue
so the other team's work is left untouched.
Args:
target (str): ``"red"`` or ``"blue"`` — which team must redo work.
Returns:
None
"""
target_state = TestState.red_executing if target == "red" else TestState.blue_evaluating
self._transition(target_state)
# Both leads must re-vote once the test reaches in_review again —
# clear decisions but keep notes (context for the team doing rework).
self.red_validation_status = None
self.red_validated_by = None
self.red_validated_at = None
self.blue_validation_status = None
self.blue_validated_by = None
self.blue_validated_at = None
self.paused_at = None
if target == "red":
self.red_started_at = datetime.utcnow()
self.red_paused_seconds = 0
else:
self.blue_started_at = datetime.utcnow()
self.blue_paused_seconds = 0
self._events.append(DomainEvent("dispute_resolved_to_rework", {"target": target}))
# -- Private ------------------------------------------------------- # -- Private -------------------------------------------------------
def _auto_resume(self) -> int: def _auto_resume(self) -> int:
@@ -694,7 +735,14 @@ class TestEntity:
# Define function _check_dual_validation # Define function _check_dual_validation
def _check_dual_validation(self) -> None: def _check_dual_validation(self) -> None:
"""Advance the test state once both leads have voted.""" """Advance the test state once enough leads have voted.
A genuine conflict (one lead's *recorded* decision disagreeing with
the other's) routes to ``disputed`` — there are two opinions to
reconcile. A lone rejection while the other side hasn't voted yet
isn't a conflict (nothing to disagree with), so it still vetoes
straight to ``rejected`` without waiting for the second vote.
"""
r, b = self.red_validation_status, self.blue_validation_status r, b = self.red_validation_status, self.blue_validation_status
if r == "approved" and b == "approved": if r == "approved" and b == "approved":
@@ -702,7 +750,16 @@ class TestEntity:
# Call self._events.append() # Call self._events.append()
self._events.append(DomainEvent("dual_validation_approved")) self._events.append(DomainEvent("dual_validation_approved"))
elif r == "rejected" or b == "rejected": elif r == "rejected" and b == "rejected":
# Any rejection is a veto — one lead can reject without waiting for the other self.state = TestState.rejected
self._events.append(DomainEvent("dual_validation_rejected"))
elif (r == "approved" and b == "rejected") or (r == "rejected" and b == "approved"):
self.state = TestState.disputed
self._events.append(DomainEvent("dual_validation_disputed"))
elif r == "rejected" or b == "rejected":
# One side rejected while the other hasn't voted yet — no
# disagreement to dispute yet, just a straightforward veto.
self.state = TestState.rejected self.state = TestState.rejected
self._events.append(DomainEvent("dual_validation_rejected")) self._events.append(DomainEvent("dual_validation_rejected"))
+24
View File
@@ -17,6 +17,7 @@ POST /tests/{id}/submit-blue — blue_evaluating → blue_review
POST /tests/{id}/review-blue — assigned Blue Lead approves/reopens/flags gap POST /tests/{id}/review-blue — assigned Blue Lead approves/reopens/flags gap
POST /tests/{id}/validate-red — Red Lead validates POST /tests/{id}/validate-red — Red Lead validates
POST /tests/{id}/validate-blue — Blue Lead validates POST /tests/{id}/validate-blue — Blue Lead validates
POST /tests/{id}/resolve-dispute — approver flips to reject, routes to red/blue queue
POST /tests/{id}/reopen — rejected → draft POST /tests/{id}/reopen — rejected → draft
GET /tests/{id}/timeline — audit-log history for this test GET /tests/{id}/timeline — audit-log history for this test
@@ -69,6 +70,7 @@ from app.schemas.test import (
TestRedUpdate, TestRedUpdate,
TestRedValidate, TestRedValidate,
TestRemediationUpdate, TestRemediationUpdate,
TestResolveDispute,
TestUpdate, TestUpdate,
) )
@@ -143,6 +145,7 @@ from app.services.test_workflow_service import (
start_blue_work as wf_start_blue_work, start_blue_work as wf_start_blue_work,
validate_as_red_lead as wf_validate_red, validate_as_red_lead as wf_validate_red,
validate_as_blue_lead as wf_validate_blue, validate_as_blue_lead as wf_validate_blue,
resolve_dispute as wf_resolve_dispute,
reopen_test as wf_reopen, reopen_test as wf_reopen,
handle_remediation_completed as wf_handle_remediation, handle_remediation_completed as wf_handle_remediation,
get_retest_chain as wf_get_retest_chain, get_retest_chain as wf_get_retest_chain,
@@ -1158,6 +1161,27 @@ def validate_blue(
return test return test
# ---------------------------------------------------------------------------
# POST /tests/{id}/resolve-dispute — approver flips to reject, picks a queue
# ---------------------------------------------------------------------------
@router.post("/{test_id}/resolve-dispute", response_model=TestOut)
def resolve_dispute(
test_id: uuid.UUID,
payload: TestResolveDispute,
db: Session = Depends(get_db),
current_user: User = Depends(require_any_role("red_lead", "blue_lead", "admin")),
) -> TestOut:
"""The lead who approved flips their vote to reject, choosing which team must redo the work."""
test = crud_get_test_with_technique(db, test_id)
with UnitOfWork(db) as uow:
test = wf_resolve_dispute(db, test, current_user, payload.target_team, notes=payload.notes)
uow.commit()
db.refresh(test)
return test
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# POST /tests/{id}/reopen — rejected → draft # POST /tests/{id}/reopen — rejected → draft
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
+10
View File
@@ -125,6 +125,16 @@ class TestBlueValidate(BaseModel):
blue_validation_notes: str | None = None blue_validation_notes: str | None = None
# ── Dispute resolution ───────────────────────────────────────────────
class TestResolveDispute(BaseModel):
"""Payload sent by the approving lead flipping their vote to reject."""
target_team: str # "red" | "blue" — which team must redo the work
notes: str | None = None
# ── Red Lead review gate (pre-Blue-Team) ──────────────────────────── # ── Red Lead review gate (pre-Blue-Team) ────────────────────────────
+86 -2
View File
@@ -33,7 +33,11 @@ from app.models.evidence import Evidence
from app.models.test import Test from app.models.test import Test
from app.models.user import User from app.models.user import User
from app.services.audit_service import log_action from app.services.audit_service import log_action
from app.services.notification_service import notify_test_state_change, create_notification from app.services.notification_service import (
notify_test_state_change,
create_notification,
notify_role_with_email,
)
# Assign logger = logging.getLogger(__name__) # Assign logger = logging.getLogger(__name__)
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -1063,6 +1067,22 @@ def _dispatch_dual_validation_effects(
elif event.name == "dual_validation_disputed": elif event.name == "dual_validation_disputed":
# Notify the lead who APPROVED asking them to review the rejection # Notify the lead who APPROVED asking them to review the rejection
_notify_validation_conflict(db, test, actor) _notify_validation_conflict(db, test, actor)
# Notify managers too, in case the dispute stalls and needs escalation
try:
notify_role_with_email(
db,
role="manager",
type="validation_disputed",
title="Validation dispute needs oversight",
message=(
f'Test "{test.name}" has a validation dispute — one lead approved, '
f'the other rejected. Escalate if it does not resolve.'
),
entity_type="test",
entity_id=test.id,
)
except Exception as e:
logger.warning("Manager dispute notification failed for test %s: %s", test.id, e, exc_info=True)
def _notify_validation_conflict(db: Session, test: Test, actor: User | None) -> None: def _notify_validation_conflict(db: Session, test: Test, actor: User | None) -> None:
@@ -1105,7 +1125,7 @@ def _notify_validation_conflict(db: Session, test: Test, actor: User | None) ->
f"or contact {rejector_role} to resolve the disagreement." f"or contact {rejector_role} to resolve the disagreement."
), ),
entity_type="test", entity_type="test",
entity_id=str(test.id), entity_id=test.id,
) )
except Exception as e: except Exception as e:
logger.warning( logger.warning(
@@ -1114,6 +1134,70 @@ def _notify_validation_conflict(db: Session, test: Test, actor: User | None) ->
) )
def resolve_dispute(db: Session, test: Test, user: User, target_team: str, notes: str | None = None) -> Test:
"""Resolve a disputed test by flipping the approving vote to reject.
Called by the lead who originally approved, now agreeing with the other
lead's rejection. Unlike a plain rejection, the flipping lead identifies
WHICH team's work needs redoing — the test routes straight back to that
team's active queue (red_executing or blue_evaluating) instead of the
generic 'rejected' state that would force a full draft restart for both
teams.
"""
if target_team not in ("red", "blue"):
raise InvalidOperationError("target_team must be 'red' or 'blue'")
if user.role != "admin":
is_red_approver = user.role == "red_lead" and test.red_validation_status == "approved"
is_blue_approver = user.role == "blue_lead" and test.blue_validation_status == "approved"
if not (is_red_approver or is_blue_approver):
raise InvalidOperationError(
"Only the lead who approved can flip their vote to resolve this dispute"
)
entity = TestEntity.from_orm(test)
entity.resolve_dispute_reject(target_team)
entity.apply_to(test)
if target_team == "blue":
test.blue_work_started_at = None # split responsibility: entity doesn't own this field
db.flush()
new_state = "red_executing" if target_team == "red" else "blue_evaluating"
log_action(
db, user_id=user.id, action="resolve_dispute",
entity_type="test", entity_id=test.id,
details={"target_team": target_team, "notes": notes, "test_name": test.name},
)
try:
notify_test_state_change(db, test, new_state)
except Exception as e:
logger.warning("Notification failed for test %s: %s", test.id, e, exc_info=True)
operator_id = test.red_tech_assignee if target_team == "red" else test.blue_tech_assignee
if operator_id and notes:
try:
create_notification(
db, user_id=operator_id, type="test_reopened",
title="Test sent back for rework (validation dispute)",
message=f'Test "{test.name}" needs rework: {notes[:200]}',
entity_type="test", entity_id=test.id,
)
except Exception as e:
logger.warning("Notification failed for test %s: %s", test.id, e, exc_info=True)
try:
from app.services.jira_service import push_test_event
push_test_event(db, test, user, new_state)
except Exception as e:
logger.warning("Jira push failed for test %s: %s", test.id, e, exc_info=True)
return test
# Define function handle_remediation_completed # Define function handle_remediation_completed
def handle_remediation_completed(db: Session, test: Test, user: User) -> Test | None: def handle_remediation_completed(db: Session, test: Test, user: User) -> Test | None:
"""Create a re-test when remediation is completed. """Create a re-test when remediation is completed.
@@ -0,0 +1,114 @@
"""HTTP-level tests for dispute resolution: POST /tests/{id}/resolve-dispute
and the manager notification fired when a test enters 'disputed'.
"""
import uuid
import pytest
from app.models.evidence import Evidence
from app.models.enums import TeamSide
def _add_evidence(db, test_id, team: TeamSide):
ev = Evidence(
test_id=uuid.UUID(test_id),
file_name="proof.txt",
file_path="s3://bucket/proof.txt",
sha256_hash="a" * 64,
team=team,
)
db.add(ev)
db.commit()
@pytest.fixture
def technique(api, auth_headers):
resp = api(
"post", "/api/v1/techniques", auth_headers,
json={"mitre_id": "T1059.200", "name": "Command Line"},
)
assert resp.status_code == 201, resp.text
return resp.json()["id"]
def _reach_disputed(client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, technique):
"""Drive a fresh test all the way to disputed: red approves, blue rejects."""
resp = api(
"post", "/api/v1/tests", auth_headers,
json={"technique_id": technique, "name": "Dispute test"},
)
test_id = resp.json()["id"]
api("post", f"/api/v1/tests/{test_id}/start-execution", red_tech_headers)
_add_evidence(db, test_id, TeamSide.red)
api("post", f"/api/v1/tests/{test_id}/submit-red", red_tech_headers)
api("post", f"/api/v1/tests/{test_id}/review-red", red_lead_headers, json={"decision": "approve"})
api("post", f"/api/v1/tests/{test_id}/start-blue-work", blue_tech_headers)
_add_evidence(db, test_id, TeamSide.blue)
api("post", f"/api/v1/tests/{test_id}/submit-blue", blue_tech_headers)
api("post", f"/api/v1/tests/{test_id}/review-blue", blue_lead_headers, json={"decision": "approve"})
red_vote = api(
"post", f"/api/v1/tests/{test_id}/validate-red", red_lead_headers,
json={"red_validation_status": "approved"},
)
assert red_vote.status_code == 200, red_vote.text
blue_vote = api(
"post", f"/api/v1/tests/{test_id}/validate-blue", blue_lead_headers,
json={"blue_validation_status": "rejected", "blue_validation_notes": "Detection insufficient"},
)
assert blue_vote.status_code == 200, blue_vote.text
assert blue_vote.json()["state"] == "disputed"
return test_id
def test_manager_notified_on_dispute(
client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, manager_headers, manager_user, technique,
):
_reach_disputed(client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, technique)
resp = api("get", "/api/v1/notifications", manager_headers)
assert resp.status_code == 200
notifications = resp.json()
assert any(n["type"] == "validation_disputed" for n in notifications)
def test_resolve_dispute_forbidden_for_non_approver(
client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, technique,
):
"""Blue Lead rejected (didn't approve) — they cannot flip the vote here."""
test_id = _reach_disputed(client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, technique)
resp = api(
"post", f"/api/v1/tests/{test_id}/resolve-dispute", blue_lead_headers,
json={"target_team": "blue"},
)
assert resp.status_code == 400
def test_resolve_dispute_routes_to_red_queue(
client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, technique,
):
"""Red Lead approved; flips to reject and sends it to the Red queue."""
test_id = _reach_disputed(client, db, api, auth_headers, red_tech_headers, red_lead_headers,
blue_tech_headers, blue_lead_headers, technique)
resp = api(
"post", f"/api/v1/tests/{test_id}/resolve-dispute", red_lead_headers,
json={"target_team": "red", "notes": "redo the attack with more detail"},
)
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["state"] == "red_executing"
assert body["red_validation_status"] is None
assert body["blue_validation_status"] is None
+72
View File
@@ -401,9 +401,32 @@ def test_dual_validation_red_rejects():
def test_dual_validation_blue_rejects(): def test_dual_validation_blue_rejects():
"""Red already approved; Blue then rejects — a genuine conflict, not a veto."""
e = _entity("in_review") e = _entity("in_review")
e.validate_red("approved", by=uuid.uuid4()) e.validate_red("approved", by=uuid.uuid4())
e.validate_blue("rejected", by=uuid.uuid4()) e.validate_blue("rejected", by=uuid.uuid4())
assert e.state == TestState.disputed
assert any(ev.name == "dual_validation_disputed" for ev in e.events)
def test_dual_validation_blue_approved_then_red_rejects():
"""Blue already approved; Red then rejects — also a genuine conflict."""
e = _entity("in_review")
e.validate_blue("approved", by=uuid.uuid4())
e.validate_red("rejected", by=uuid.uuid4())
assert e.state == TestState.disputed
def test_dual_validation_both_rejected_from_disputed():
"""A disputed test where the approving lead flips to reject (via the
plain validate call, not resolve_dispute_reject) still lands on the
generic terminal 'rejected' state — both leads now agree it's bad."""
e = _entity(
"disputed",
red_validation_status="approved",
blue_validation_status="rejected",
)
e.validate_red("rejected", by=uuid.uuid4())
assert e.state == TestState.rejected assert e.state == TestState.rejected
@@ -561,3 +584,52 @@ def test_is_terminal():
assert _entity("validated").is_terminal is True assert _entity("validated").is_terminal is True
assert _entity("rejected").is_terminal is False assert _entity("rejected").is_terminal is False
assert _entity("draft").is_terminal is False assert _entity("draft").is_terminal is False
# ── 13. resolve_dispute_reject ──────────────────────────────────────
def test_resolve_dispute_reject_to_red():
e = _entity(
"disputed",
red_validation_status="approved",
red_validated_by=uuid.uuid4(),
red_validated_at=datetime.utcnow(),
blue_validation_status="rejected",
blue_validated_by=uuid.uuid4(),
blue_validated_at=datetime.utcnow(),
red_paused_seconds=50,
)
e.resolve_dispute_reject("red")
assert e.state == TestState.red_executing
assert e.red_validation_status is None
assert e.red_validated_by is None
assert e.blue_validation_status is None
assert e.blue_validated_by is None
assert e.red_started_at is not None
assert e.red_paused_seconds == 0
assert any(ev.name == "dispute_resolved_to_rework" and ev.payload["target"] == "red" for ev in e.events)
def test_resolve_dispute_reject_to_blue():
e = _entity(
"disputed",
blue_validation_status="approved",
blue_validated_by=uuid.uuid4(),
red_validation_status="rejected",
red_validated_by=uuid.uuid4(),
blue_paused_seconds=20,
)
e.resolve_dispute_reject("blue")
assert e.state == TestState.blue_evaluating
assert e.red_validation_status is None
assert e.blue_validation_status is None
assert e.blue_started_at is not None
assert e.blue_paused_seconds == 0
assert any(ev.name == "dispute_resolved_to_rework" and ev.payload["target"] == "blue" for ev in e.events)
def test_resolve_dispute_reject_wrong_state():
e = _entity("in_review")
with pytest.raises(InvalidStateTransition):
e.resolve_dispute_reject("red")
+81 -2
View File
@@ -414,7 +414,7 @@ def test_dual_validation_blue_rejects_first(mock_log):
@patch("app.services.test_workflow_service.log_action") @patch("app.services.test_workflow_service.log_action")
def test_dual_validation_red_approves_blue_rejects(mock_log): def test_dual_validation_red_approves_blue_rejects(mock_log):
"""Red approves, then blue rejects -> rejected.""" """Red approves, then blue rejects -> genuine conflict -> disputed."""
test = _make_test(TestState.in_review) test = _make_test(TestState.in_review)
red_lead = _make_user("red_lead") red_lead = _make_user("red_lead")
blue_lead = _make_user("blue_lead") blue_lead = _make_user("blue_lead")
@@ -424,7 +424,7 @@ def test_dual_validation_red_approves_blue_rejects(mock_log):
assert test.state == TestState.in_review # waiting for blue assert test.state == TestState.in_review # waiting for blue
validate_as_blue_lead(db, test, blue_lead, "rejected", "Bad detection") validate_as_blue_lead(db, test, blue_lead, "rejected", "Bad detection")
assert test.state == TestState.rejected assert test.state == TestState.disputed
# =========================================================================== # ===========================================================================
@@ -687,6 +687,85 @@ class TestReviewDecisions:
assert result.system_gaps == "Missing EDR agent on host X" assert result.system_gaps == "Missing EDR agent on host X"
# ===========================================================================
# 12c. resolve_dispute — flip approver's vote to reject, route to a team
# ===========================================================================
class TestResolveDispute:
@patch("app.services.test_workflow_service.log_action")
def test_red_lead_approver_flips_to_red_queue(self, mock_log):
test = _make_test(
TestState.disputed,
red_validation_status="approved",
blue_validation_status="rejected",
)
red_lead = _make_user("red_lead")
db = _make_db()
from app.services.test_workflow_service import resolve_dispute
result = resolve_dispute(db, test, red_lead, "red", notes="redo the attack")
assert result.state == TestState.red_executing
assert result.red_validation_status is None
assert result.blue_validation_status is None
@patch("app.services.test_workflow_service.log_action")
def test_blue_lead_approver_flips_to_blue_queue(self, mock_log):
test = _make_test(
TestState.disputed,
blue_validation_status="approved",
red_validation_status="rejected",
)
blue_lead = _make_user("blue_lead")
db = _make_db()
from app.services.test_workflow_service import resolve_dispute
result = resolve_dispute(db, test, blue_lead, "blue")
assert result.state == TestState.blue_evaluating
def test_non_approver_cannot_resolve_dispute(self):
test = _make_test(
TestState.disputed,
red_validation_status="rejected",
blue_validation_status="approved",
)
red_lead = _make_user("red_lead") # red_lead REJECTED, didn't approve
db = _make_db()
from app.services.test_workflow_service import resolve_dispute
with pytest.raises(InvalidOperationError):
resolve_dispute(db, test, red_lead, "red")
def test_invalid_target_team_rejected(self):
test = _make_test(
TestState.disputed,
red_validation_status="approved",
blue_validation_status="rejected",
)
red_lead = _make_user("red_lead")
db = _make_db()
from app.services.test_workflow_service import resolve_dispute
with pytest.raises(InvalidOperationError):
resolve_dispute(db, test, red_lead, "purple")
@patch("app.services.test_workflow_service.log_action")
def test_admin_can_resolve_dispute_regardless_of_vote(self, mock_log):
test = _make_test(
TestState.disputed,
red_validation_status="approved",
blue_validation_status="rejected",
)
admin = _make_user("admin")
db = _make_db()
from app.services.test_workflow_service import resolve_dispute
result = resolve_dispute(db, test, admin, "red")
assert result.state == TestState.red_executing
# =========================================================================== # ===========================================================================
# 13. select_reviewer — load-balanced reviewer assignment # 13. select_reviewer — load-balanced reviewer assignment
# =========================================================================== # ===========================================================================