diff --git a/backend/app/schemas/test.py b/backend/app/schemas/test.py index 36302fd..4fedd24 100644 --- a/backend/app/schemas/test.py +++ b/backend/app/schemas/test.py @@ -4,15 +4,29 @@ import uuid # Import datetime from datetime -from datetime import datetime +from datetime import datetime, timezone -from pydantic import BaseModel, ConfigDict, model_validator +from pydantic import BaseModel, ConfigDict, field_validator, model_validator # Import DataClassification from app.domain.enums from app.domain.enums import AttackSuccessResult, ContainmentResult, DataClassification from app.models.enums import TestResult, TestState from app.schemas.evidence import EvidenceOut + +def _to_naive_utc(v: datetime | None) -> datetime | None: + """Normalize an incoming datetime to naive UTC. + + Every timestamp column in this app is naive-UTC (set via + ``datetime.utcnow()`` server-side). Clients are expected to send a + proper UTC ISO string (with a ``Z``/offset), but if one slips through + tz-aware, convert it rather than let it get silently mis-stored as if + its clock digits were already UTC. + """ + if v is not None and v.tzinfo is not None: + return v.astimezone(timezone.utc).replace(tzinfo=None) + return v + # ── Create ────────────────────────────────────────────────────────── @@ -85,6 +99,8 @@ class TestRedUpdate(BaseModel): execution_start_time: datetime | None = None execution_end_time: datetime | None = None + _normalize_datetimes = field_validator("execution_start_time", "execution_end_time")(_to_naive_utc) + # ── Blue Team update ─────────────────────────────────────────────── @@ -100,6 +116,8 @@ class TestBlueUpdate(BaseModel): # Assign blue_summary = None blue_summary: str | None = None + _normalize_datetimes = field_validator("detection_time", "containment_time")(_to_naive_utc) + # ── Round history (archived on reopen) ───────────────────────────── diff --git a/backend/app/services/jira_service.py b/backend/app/services/jira_service.py index 7604a4e..e07b79d 100644 --- a/backend/app/services/jira_service.py +++ b/backend/app/services/jira_service.py @@ -1107,6 +1107,7 @@ def push_round_archived(db: Session, test, actor, *, round_data) -> None: comment = "\n".join(lines) jira.issue_add_comment(link.jira_issue_key, comment) + _add_jira_label(jira, link.jira_issue_key, "reopened") link.last_synced_at = datetime.utcnow() db.flush() logger.info( @@ -1117,6 +1118,19 @@ def push_round_archived(db: Session, test, actor, *, round_data) -> None: logger.warning("Failed to push round-archived comment for test %s: %s", test.id, exc, exc_info=True) +def _add_jira_label(jira, issue_key: str, label: str) -> None: + """Add *label* to an issue's labels without clobbering the existing ones. + + Jira's update API takes the whole ``labels`` array, so adding one means + read-modify-write: fetch the current list, append if missing, write it + back. Failures here are logged and swallowed by the caller. + """ + issue = jira.issue(issue_key, fields="labels") + current = (issue.get("fields") or {}).get("labels") or [] + if label not in current: + jira.update_issue_field(issue_key, fields={"labels": current + [label]}) + + def push_bt_work_started(db: Session, test, actor) -> None: """Transition the Jira issue to "In Progress" when a blue tech picks up a queued test to start evaluating. @@ -1228,19 +1242,45 @@ def _update_test_fields(db: Session, test: Test, fields: dict) -> None: ) -def push_rt_started(db: Session, test: Test) -> None: - """Set the RT Start Date field — called when the operator hits Start Execution.""" - _update_test_fields(db, test, { - JIRA_FIELD_RT_START_DATE: datetime.utcnow().strftime("%Y-%m-%d"), - }) - - def push_rt_submitted(db: Session, test: Test) -> None: - """Set the RT End Date + Attack Success fields — called when Red submits for review.""" - fields: dict = {JIRA_FIELD_RT_END_DATE: datetime.utcnow().strftime("%Y-%m-%d")} + """Set RT Start Date, RT End Date, and Attack Success — called when Red submits for review. + + RT Start Date always reflects the *first* round's execution start (from + round_number=1 in round_history if this test has been reopened, else the + live field — this is round 1 in that case). RT End Date and Attack + Success always reflect the *current* round, since they're read live off + the test each time this runs and get pushed again on every resubmit. + Previously this pushed ``datetime.utcnow()`` (click time) instead of the + operator-entered execution window — fixed to read the real fields. + """ + from app.models.test_round_history import TestRoundHistory + + fields: dict = {} + + first_round = ( + db.query(TestRoundHistory) + .filter( + TestRoundHistory.test_id == test.id, + TestRoundHistory.team == "red", + TestRoundHistory.round_number == 1, + ) + .first() + ) + first_start = ( + first_round.execution_start_time + if first_round and first_round.execution_start_time + else test.execution_start_time + ) + if first_start: + fields[JIRA_FIELD_RT_START_DATE] = first_start.strftime("%Y-%m-%d") + + if test.execution_end_time: + fields[JIRA_FIELD_RT_END_DATE] = test.execution_end_time.strftime("%Y-%m-%d") + attack_success = _enum_value(test.attack_success) if attack_success in _ATTACK_SUCCESS_TO_JIRA: fields[JIRA_FIELD_ATTACK_SUCCESS] = _select_field(_ATTACK_SUCCESS_TO_JIRA[attack_success]) + _update_test_fields(db, test, fields) diff --git a/backend/app/services/test_workflow_service.py b/backend/app/services/test_workflow_service.py index cad8a08..05e67af 100644 --- a/backend/app/services/test_workflow_service.py +++ b/backend/app/services/test_workflow_service.py @@ -243,12 +243,6 @@ def start_execution(db: Session, test: Test, user: User) -> Test: except Exception as e: logger.warning("Jira push failed for test %s: %s", test.id, e, exc_info=True) - try: - from app.services.jira_service import push_rt_started - push_rt_started(db, test) - except Exception as e: - logger.warning("Jira RT start date push failed for test %s: %s", test.id, e, exc_info=True) - return test diff --git a/backend/tests/test_jira_service.py b/backend/tests/test_jira_service.py index aa4692f..d9b1fe8 100644 --- a/backend/tests/test_jira_service.py +++ b/backend/tests/test_jira_service.py @@ -53,9 +53,12 @@ def _make_test(**overrides): t.attack_success = None t.detection_result = None t.containment_result = None + t.execution_start_time = None t.execution_end_time = None t.detection_time = None t.containment_time = None + t.red_round_number = 1 + t.blue_round_number = 1 # _build_state_comment() appends these raw (not via an f-string) when # truthy, so an un-nulled MagicMock attribute breaks "\n".join(lines). t.red_summary = None @@ -143,16 +146,55 @@ def test_push_round_archived_posts_comment_without_changing_status(mock_get_clie review_notes="Evidence was incomplete, redo with screenshots", ) mock_jira = MagicMock() + mock_jira.issue.return_value = {"fields": {"labels": []}} mock_get_client.return_value = mock_jira jira_service.push_round_archived(db, test, MagicMock(username="redlead"), round_data=round_data) mock_jira.set_issue_status.assert_not_called() mock_jira.issue_add_comment.assert_called_once() - comment = mock_jira.issue_add_comment.call_args[0][1] - assert "Round 1" in comment - assert "Got a shell." in comment - assert "Evidence was incomplete" in comment + + +@patch("app.services.jira_service.has_admin_jira_configured", return_value=True) +@patch("app.services.jira_service.get_admin_jira_client") +def test_push_round_archived_adds_reopened_label_without_clobbering_others(mock_get_client, mock_configured, db): + from app.models.test_round_history import TestRoundHistory + + test = _make_test() + link = _make_link(test.id) + db.add(link) + db.commit() + + round_data = TestRoundHistory(test_id=test.id, team="blue", round_number=1) + mock_jira = MagicMock() + mock_jira.issue.return_value = {"fields": {"labels": ["security-review"]}} + mock_get_client.return_value = mock_jira + + jira_service.push_round_archived(db, test, MagicMock(username="bluelead"), round_data=round_data) + + mock_jira.update_issue_field.assert_called_once_with( + link.jira_issue_key, fields={"labels": ["security-review", "reopened"]}, + ) + + +@patch("app.services.jira_service.has_admin_jira_configured", return_value=True) +@patch("app.services.jira_service.get_admin_jira_client") +def test_push_round_archived_does_not_duplicate_reopened_label(mock_get_client, mock_configured, db): + from app.models.test_round_history import TestRoundHistory + + test = _make_test() + link = _make_link(test.id) + db.add(link) + db.commit() + + round_data = TestRoundHistory(test_id=test.id, team="red", round_number=2) + mock_jira = MagicMock() + mock_jira.issue.return_value = {"fields": {"labels": ["reopened"]}} + mock_get_client.return_value = mock_jira + + jira_service.push_round_archived(db, test, MagicMock(username="redlead"), round_data=round_data) + + mock_jira.update_issue_field.assert_not_called() @pytest.mark.parametrize( @@ -316,30 +358,16 @@ def test_push_bt_work_started_sets_in_progress(mock_get_client, mock_configured, mock_jira.assign_issue.assert_called_once_with(link.jira_issue_key, account_id="bob-account-id") -@patch("app.services.jira_service.has_admin_jira_configured", return_value=True) -@patch("app.services.jira_service.get_admin_jira_client") -def test_push_rt_started_sets_date_field(mock_get_client, mock_configured, db): - mock_jira = MagicMock() - mock_get_client.return_value = mock_jira - test = _make_test() - link = _make_link(test.id) - db.add(link) - db.commit() - - jira_service.push_rt_started(db, test) - - mock_jira.update_issue_field.assert_called_once() - args, kwargs = mock_jira.update_issue_field.call_args - assert args[0] == link.jira_issue_key - assert jira_service.JIRA_FIELD_RT_START_DATE in kwargs["fields"] - - @patch("app.services.jira_service.has_admin_jira_configured", return_value=True) @patch("app.services.jira_service.get_admin_jira_client") def test_push_rt_submitted_includes_attack_success(mock_get_client, mock_configured, db): mock_jira = MagicMock() mock_get_client.return_value = mock_jira - test = _make_test(attack_success=MagicMock(value="successful")) + test = _make_test( + attack_success=MagicMock(value="successful"), + execution_start_time=datetime(2026, 2, 1, 8, 0, 0), + execution_end_time=datetime(2026, 2, 1, 9, 0, 0), + ) link = _make_link(test.id) db.add(link) db.commit() @@ -351,6 +379,70 @@ def test_push_rt_submitted_includes_attack_success(mock_get_client, mock_configu assert fields[jira_service.JIRA_FIELD_ATTACK_SUCCESS] == {"value": "Yes"} +@patch("app.services.jira_service.has_admin_jira_configured", return_value=True) +@patch("app.services.jira_service.get_admin_jira_client") +def test_push_rt_submitted_uses_execution_times_not_click_time(mock_get_client, mock_configured, db): + """Regression: this used to push datetime.utcnow() (whenever the submit + button was clicked) instead of the operator-entered execution window.""" + mock_jira = MagicMock() + mock_get_client.return_value = mock_jira + test = _make_test( + execution_start_time=datetime(2026, 3, 1, 9, 0, 0), + execution_end_time=datetime(2026, 3, 1, 11, 0, 0), + ) + link = _make_link(test.id) + db.add(link) + db.commit() + + jira_service.push_rt_submitted(db, test) + + fields = mock_jira.update_issue_field.call_args[1]["fields"] + assert fields[jira_service.JIRA_FIELD_RT_START_DATE] == "2026-03-01" + assert fields[jira_service.JIRA_FIELD_RT_END_DATE] == "2026-03-01" + + +@patch("app.services.jira_service.has_admin_jira_configured", return_value=True) +@patch("app.services.jira_service.get_admin_jira_client") +def test_push_rt_submitted_rt_start_date_survives_reopen(mock_get_client, mock_configured, db): + """RT Start Date must keep showing round 1's execution start even after + a reopen resets the live execution_start_time for round 2 — otherwise + Jira loses track of when the test was originally attempted.""" + from app.models.technique import Technique + from app.models.test import Test as TestModel + from app.models.test_round_history import TestRoundHistory + + # A real, committed Test row is required here (unlike the other tests in + # this file) because test_round_history.test_id has a real FK to tests.id. + technique = Technique(mitre_id="T9997", name="RT Date Test Technique", tactic="execution", platforms=["linux"]) + db.add(technique) + db.commit() + real_test = TestModel(technique_id=technique.id, name="RT date fixture") + db.add(real_test) + db.commit() + + mock_jira = MagicMock() + mock_get_client.return_value = mock_jira + test = _make_test( + id=real_test.id, + red_round_number=2, + execution_start_time=datetime(2026, 3, 5, 8, 0, 0), # round 2's start + execution_end_time=datetime(2026, 3, 5, 10, 0, 0), + ) + link = _make_link(test.id) + db.add(link) + db.add(TestRoundHistory( + test_id=test.id, team="red", round_number=1, + execution_start_time=datetime(2026, 3, 1, 9, 0, 0), # round 1's start + )) + db.commit() + + jira_service.push_rt_submitted(db, test) + + fields = mock_jira.update_issue_field.call_args[1]["fields"] + assert fields[jira_service.JIRA_FIELD_RT_START_DATE] == "2026-03-01" + assert fields[jira_service.JIRA_FIELD_RT_END_DATE] == "2026-03-05" + + @patch("app.services.jira_service.has_admin_jira_configured", return_value=True) @patch("app.services.jira_service.get_admin_jira_client") def test_push_bt_submitted_includes_detection_and_hours(mock_get_client, mock_configured, db): diff --git a/frontend/src/pages/TestDetailPage.tsx b/frontend/src/pages/TestDetailPage.tsx index c706ac4..f493a89 100644 --- a/frontend/src/pages/TestDetailPage.tsx +++ b/frontend/src/pages/TestDetailPage.tsx @@ -44,9 +44,28 @@ import { createTemplate } from "../api/test-templates"; // ── Helpers ──────────────────────────────────────────────────────── +// The backend always stores/returns naive UTC datetimes (no "Z", no +// offset). shows/edits wall-clock time in +// the browser's own timezone, so both directions need an explicit +// UTC <-> local conversion — treating the raw digits as interchangeable +// silently shifts every value by the browser's UTC offset. function isoToDatetimeLocal(iso: string | null): string { if (!iso) return ""; - return iso.replace("Z", "").replace(/\.\d+$/, "").slice(0, 16); + const utcStr = iso.endsWith("Z") || iso.includes("+") ? iso : iso + "Z"; + const d = new Date(utcStr); + if (isNaN(d.getTime())) return ""; + const pad = (n: number) => String(n).padStart(2, "0"); + return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}`; +} + +function datetimeLocalToIso(local: string): string | undefined { + if (!local) return undefined; + // New Date() on a plain "YYYY-MM-DDTHH:mm" string (no zone designator) + // parses it as local time per the ECMA-262 date string spec — exactly + // the wall-clock time the user picked. + const d = new Date(local); + if (isNaN(d.getTime())) return undefined; + return d.toISOString(); } // ── Page Component ───────────────────────────────────────────────── @@ -205,8 +224,8 @@ export default function TestDetailPage() { tool_used: redDraft.tool_used || undefined, attack_success: redDraft.attack_success || undefined, red_summary: redDraft.red_summary || undefined, - execution_start_time: redDraft.execution_start_time || undefined, - execution_end_time: redDraft.execution_end_time || undefined, + execution_start_time: datetimeLocalToIso(redDraft.execution_start_time), + execution_end_time: datetimeLocalToIso(redDraft.execution_end_time), }), onSuccess: () => { invalidateAll(); @@ -220,8 +239,8 @@ export default function TestDetailPage() { updateTestBlue(testId!, { detection_result: (blueDraft.detection_result as TestResult) || undefined, containment_result: (blueDraft.containment_result as ContainmentResult) || undefined, - detection_time: blueDraft.detection_time || undefined, - containment_time: blueDraft.containment_time || undefined, + detection_time: datetimeLocalToIso(blueDraft.detection_time), + containment_time: datetimeLocalToIso(blueDraft.containment_time), blue_summary: blueDraft.blue_summary || undefined, }), onSuccess: () => {