diff --git a/backend/app/domain/test_entity.py b/backend/app/domain/test_entity.py index 6748d9f..fca1c03 100644 --- a/backend/app/domain/test_entity.py +++ b/backend/app/domain/test_entity.py @@ -83,7 +83,10 @@ VALID_TRANSITIONS: dict[TestState, list[TestState]] = { TestState.blue_evaluating: [TestState.blue_review], TestState.blue_review: [TestState.in_review, TestState.blue_evaluating], 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.validated: [], } @@ -627,6 +630,44 @@ class TestEntity: # Call self._events.append() 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 ------------------------------------------------------- def _auto_resume(self) -> int: @@ -694,7 +735,14 @@ class TestEntity: # Define function _check_dual_validation 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 if r == "approved" and b == "approved": @@ -702,7 +750,16 @@ class TestEntity: # Call self._events.append() self._events.append(DomainEvent("dual_validation_approved")) - elif r == "rejected" or b == "rejected": - # Any rejection is a veto — one lead can reject without waiting for the other + elif r == "rejected" and b == "rejected": + 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._events.append(DomainEvent("dual_validation_rejected")) diff --git a/backend/app/routers/tests.py b/backend/app/routers/tests.py index 7d2b930..4394df4 100644 --- a/backend/app/routers/tests.py +++ b/backend/app/routers/tests.py @@ -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}/validate-red — Red 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 GET /tests/{id}/timeline — audit-log history for this test @@ -69,6 +70,7 @@ from app.schemas.test import ( TestRedUpdate, TestRedValidate, TestRemediationUpdate, + TestResolveDispute, TestUpdate, ) @@ -143,6 +145,7 @@ from app.services.test_workflow_service import ( start_blue_work as wf_start_blue_work, validate_as_red_lead as wf_validate_red, validate_as_blue_lead as wf_validate_blue, + resolve_dispute as wf_resolve_dispute, reopen_test as wf_reopen, handle_remediation_completed as wf_handle_remediation, get_retest_chain as wf_get_retest_chain, @@ -1158,6 +1161,27 @@ def validate_blue( 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 # --------------------------------------------------------------------------- diff --git a/backend/app/schemas/test.py b/backend/app/schemas/test.py index a7d3369..35d996e 100644 --- a/backend/app/schemas/test.py +++ b/backend/app/schemas/test.py @@ -125,6 +125,16 @@ class TestBlueValidate(BaseModel): 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) ──────────────────────────── diff --git a/backend/app/services/test_workflow_service.py b/backend/app/services/test_workflow_service.py index fad2f51..4027ff5 100644 --- a/backend/app/services/test_workflow_service.py +++ b/backend/app/services/test_workflow_service.py @@ -33,7 +33,11 @@ from app.models.evidence import Evidence from app.models.test import Test from app.models.user import User 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__) logger = logging.getLogger(__name__) @@ -1063,6 +1067,22 @@ def _dispatch_dual_validation_effects( elif event.name == "dual_validation_disputed": # Notify the lead who APPROVED asking them to review the rejection _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: @@ -1105,7 +1125,7 @@ def _notify_validation_conflict(db: Session, test: Test, actor: User | None) -> f"or contact {rejector_role} to resolve the disagreement." ), entity_type="test", - entity_id=str(test.id), + entity_id=test.id, ) except Exception as e: 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 def handle_remediation_completed(db: Session, test: Test, user: User) -> Test | None: """Create a re-test when remediation is completed. diff --git a/backend/tests/test_resolve_dispute_router.py b/backend/tests/test_resolve_dispute_router.py new file mode 100644 index 0000000..75119dd --- /dev/null +++ b/backend/tests/test_resolve_dispute_router.py @@ -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 diff --git a/backend/tests/test_test_entity.py b/backend/tests/test_test_entity.py index 8afe59c..4226a3b 100644 --- a/backend/tests/test_test_entity.py +++ b/backend/tests/test_test_entity.py @@ -401,9 +401,32 @@ def test_dual_validation_red_rejects(): def test_dual_validation_blue_rejects(): + """Red already approved; Blue then rejects — a genuine conflict, not a veto.""" e = _entity("in_review") e.validate_red("approved", 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 @@ -561,3 +584,52 @@ def test_is_terminal(): assert _entity("validated").is_terminal is True assert _entity("rejected").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") diff --git a/backend/tests/test_workflow.py b/backend/tests/test_workflow.py index de511b1..ac46b65 100644 --- a/backend/tests/test_workflow.py +++ b/backend/tests/test_workflow.py @@ -414,7 +414,7 @@ def test_dual_validation_blue_rejects_first(mock_log): @patch("app.services.test_workflow_service.log_action") 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) red_lead = _make_user("red_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 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" +# =========================================================================== +# 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 # ===========================================================================