From be2a3fdcedd79500ce6183befa24a4b438710f8b Mon Sep 17 00:00:00 2001 From: kitos Date: Fri, 10 Jul 2026 15:24:19 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20multiple=20test-workflow=20bugs=20?= =?UTF-8?q?=E2=80=94=20draft=20wipe,=20dark=20date=20pickers,=20Jira=20sta?= =?UTF-8?q?tus=20strings,=20Tempo=20gate,=20hold=20timer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix the red/blue draft form losing unsaved fields (e.g. execution_start_time) whenever an evidence upload refetches the test — the hydration effect now runs once per test, not on every refetch. - Add color-scheme: dark so native date/time picker popups match the app theme instead of rendering white. - Fix _STATE_TO_JIRA_STATUS: real Jira statuses are 'To Do' and 'Red Team test review', not 'To-Do' and 'RT Test Review' — confirmed live against the actual workflow transitions, which is why Submit to Blue Team never moved the ticket past 'In Progress'. - Tempo worklog sync was silently blocked by TEMPO_ENABLED defaulting to False with no admin-facing toggle, even after configuring a Tempo token via Settings. Now bypassed once an admin or personal token is actually configured, mirroring Jira's DB-backed enabled flag. - Hold now actually pauses the running phase timer (paused_at), and Resume accumulates the held duration into red/blue_paused_seconds — previously the timer kept counting through a hold. --- backend/app/routers/tests.py | 15 +++++ backend/app/services/jira_service.py | 6 +- backend/app/services/tempo_service.py | 15 +++-- backend/tests/test_hold_resume.py | 90 +++++++++++++++++++++++++++ backend/tests/test_jira_service.py | 6 +- backend/tests/test_tempo_service.py | 42 +++++++++++++ frontend/src/index.css | 7 +++ frontend/src/pages/TestDetailPage.tsx | 12 +++- 8 files changed, 180 insertions(+), 13 deletions(-) create mode 100644 backend/tests/test_hold_resume.py diff --git a/backend/app/routers/tests.py b/backend/app/routers/tests.py index bada35b..d8d1a35 100644 --- a/backend/app/routers/tests.py +++ b/backend/app/routers/tests.py @@ -1354,6 +1354,10 @@ def hold_test( test.hold_reason = payload.reason test.held_at = _dt.utcnow() + # A running phase timer must stop counting while on hold. + if test.state in (TestState.red_executing, TestState.blue_evaluating) and test.paused_at is None: + test.paused_at = test.held_at + log_action(db, current_user.id, "hold_test", str(test_id), {"reason": payload.reason}) db.commit() db.refresh(test) @@ -1375,6 +1379,7 @@ def resume_test( current_user: User = Depends(require_any_role_strict("red_tech", "blue_tech")), ): """Resume a test that was placed on hold.""" + from datetime import datetime as _dt from app.services.jira_service import push_hold_event test = crud_get_test_or_raise(db, test_id) @@ -1386,6 +1391,16 @@ def resume_test( test.hold_reason = None test.held_at = None + # Resume the phase timer that hold_test paused, accumulating the held + # duration the same way pause/resume-timer does. + if test.paused_at is not None: + held_seconds = max(int((_dt.utcnow() - test.paused_at).total_seconds()), 0) + if test.state == TestState.red_executing: + test.red_paused_seconds = (test.red_paused_seconds or 0) + held_seconds + elif test.state == TestState.blue_evaluating: + test.blue_paused_seconds = (test.blue_paused_seconds or 0) + held_seconds + test.paused_at = None + log_action(db, current_user.id, "resume_test", str(test_id), {}) db.commit() db.refresh(test) diff --git a/backend/app/services/jira_service.py b/backend/app/services/jira_service.py index 4fcd841..59b0208 100644 --- a/backend/app/services/jira_service.py +++ b/backend/app/services/jira_service.py @@ -379,11 +379,11 @@ _STATE_EMOJI: dict[str, str] = { # (reopen_blue_review), that's pushed as "In Progress" directly instead of # through this table — see push_bt_review_reopened(). _STATE_TO_JIRA_STATUS: dict[str, str] = { - "draft": "To-Do", + "draft": "To Do", "red_executing": "In Progress", - "red_review": "RT Test Review", + "red_review": "Red Team test review", "blue_evaluating": "Queued Blue Team", - "blue_review": "Blue Team Test Review", + "blue_review": "Blue Team test review", "in_review": "Validation", "validated": "Done", "rejected": "Rejected", diff --git a/backend/app/services/tempo_service.py b/backend/app/services/tempo_service.py index ef36309..5e10b60 100644 --- a/backend/app/services/tempo_service.py +++ b/backend/app/services/tempo_service.py @@ -6,8 +6,11 @@ Each user authenticates to Tempo with their own personal Tempo API token, stored in ``user.tempo_api_token``. This is different from the Jira API token. Obtain a Tempo token at: Jira → Apps → Tempo → Settings → API Integration. -The global ``settings.TEMPO_ENABLED`` flag acts as a kill-switch. When False, -all Tempo calls are silently skipped regardless of whether users have tokens. +The global ``settings.TEMPO_ENABLED`` env var is an optional hard kill-switch +for ops (e.g. disable entirely in a demo environment). It defaults to False, +but that default is bypassed automatically once an admin or personal Tempo +token is actually configured — configuring a token via Settings is enough +to turn Tempo sync on, same as Jira's DB-backed "enabled" toggle. What goes to Tempo ------------------ @@ -223,8 +226,12 @@ def auto_log_test_worklog( logger.debug("Skipping Tempo sync for activity_type=%s", activity_type) return None - # Global kill-switch - if not settings.TEMPO_ENABLED: + # Global kill-switch — bypassed once an admin or personal Tempo token + # is actually configured, mirroring how Jira's DB-backed "enabled" + # toggle takes priority over its env-var default. Without this, an + # admin who configures a Tempo token via Settings still gets silent + # no-ops because TEMPO_ENABLED defaults to False and has no UI toggle. + if not settings.TEMPO_ENABLED and not has_admin_tempo_configured(db) and not has_tempo_configured(user): return None # Compute duration from test timestamps when not supplied by the caller diff --git a/backend/tests/test_hold_resume.py b/backend/tests/test_hold_resume.py new file mode 100644 index 0000000..8364f58 --- /dev/null +++ b/backend/tests/test_hold_resume.py @@ -0,0 +1,90 @@ +"""Tests for POST /tests/{id}/hold and /resume — must pause/resume the phase timer.""" + +from datetime import datetime, timedelta + +from app.models.test import Test +from app.models.technique import Technique +from app.models.enums import TestState + + +def _seed_technique(db) -> Technique: + technique = Technique( + mitre_id="T9999", name="Test Technique", tactic="execution", platforms=["linux"], + ) + db.add(technique) + db.commit() + db.refresh(technique) + return technique + + +def _seed_executing_test(db, technique, created_by) -> Test: + test = Test( + technique_id=technique.id, + name="Holdable test", + created_by=created_by, + state=TestState.red_executing, + red_started_at=datetime.utcnow() - timedelta(minutes=10), + ) + db.add(test) + db.commit() + db.refresh(test) + return test + + +def test_hold_pauses_the_running_timer(client, db, red_tech_headers, red_tech_user): + technique = _seed_technique(db) + test = _seed_executing_test(db, technique, red_tech_user.id) + assert test.paused_at is None + + resp = client.post( + f"/api/v1/tests/{test.id}/hold", + json={"reason": "waiting on lab access"}, + headers=red_tech_headers, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["paused_at"] is not None + + db.refresh(test) + assert test.paused_at is not None + + +def test_resume_clears_pause_and_accumulates_seconds(client, db, red_tech_headers, red_tech_user): + technique = _seed_technique(db) + test = _seed_executing_test(db, technique, red_tech_user.id) + + resp = client.post( + f"/api/v1/tests/{test.id}/hold", + json={"reason": "waiting on lab access"}, + headers=red_tech_headers, + ) + assert resp.status_code == 200 + + resp = client.post(f"/api/v1/tests/{test.id}/resume", headers=red_tech_headers) + assert resp.status_code == 200, resp.text + assert resp.json()["paused_at"] is None + + db.refresh(test) + assert test.paused_at is None + assert test.red_paused_seconds >= 0 + + +def test_hold_does_not_double_pause_an_already_paused_timer(client, db, red_tech_headers, red_tech_user): + """If the operator already paused the timer manually, holding must not + overwrite paused_at (which would reset the elapsed-pause calculation).""" + technique = _seed_technique(db) + test = _seed_executing_test(db, technique, red_tech_user.id) + + resp = client.post(f"/api/v1/tests/{test.id}/pause-timer", headers=red_tech_headers) + assert resp.status_code == 200 + db.refresh(test) + manual_pause_time = test.paused_at + + resp = client.post( + f"/api/v1/tests/{test.id}/hold", + json={"reason": "waiting on lab access"}, + headers=red_tech_headers, + ) + assert resp.status_code == 200 + + db.refresh(test) + assert test.paused_at == manual_pause_time diff --git a/backend/tests/test_jira_service.py b/backend/tests/test_jira_service.py index 2918e63..bd658b5 100644 --- a/backend/tests/test_jira_service.py +++ b/backend/tests/test_jira_service.py @@ -119,11 +119,11 @@ def test_push_hold_event_sets_blocked(mock_get_client, mock_configured, db): @pytest.mark.parametrize( "new_state,expected_status", [ - ("draft", "To-Do"), + ("draft", "To Do"), ("red_executing", "In Progress"), - ("red_review", "RT Test Review"), + ("red_review", "Red Team test review"), ("blue_evaluating", "Queued Blue Team"), - ("blue_review", "Blue Team Test Review"), + ("blue_review", "Blue Team test review"), ("in_review", "Validation"), ("validated", "Done"), ("rejected", "Rejected"), diff --git a/backend/tests/test_tempo_service.py b/backend/tests/test_tempo_service.py index e634eb3..b456826 100644 --- a/backend/tests/test_tempo_service.py +++ b/backend/tests/test_tempo_service.py @@ -12,13 +12,55 @@ from app.services import tempo_service def test_auto_log_test_worklog_skips_when_disabled(monkeypatch, db): + """No env kill-switch, no admin token, no personal token — must no-op.""" monkeypatch.setattr("app.services.tempo_service.settings.TEMPO_ENABLED", False) test = MagicMock() test.id = uuid4() user = MagicMock() + user.tempo_api_token = None assert tempo_service.auto_log_test_worklog(db, test, user, "red_team_execution") is None +@patch("tempoapiclient.client_v4.Tempo") +def test_auto_log_test_worklog_proceeds_with_admin_token_even_when_disabled( + mock_tempo_cls, monkeypatch, db, admin_user +): + """Regression: TEMPO_ENABLED defaults False with no UI toggle — an admin + who configures a Tempo token via Settings must not be silently blocked.""" + monkeypatch.setattr("app.services.tempo_service.settings.TEMPO_ENABLED", False) + from app.models.system_config import SystemConfig + db.add(SystemConfig(key="tempo.admin_token", value="admin-tempo-token")) + + test_id = uuid4() + link = JiraLink( + entity_type=JiraLinkEntityType.test, + entity_id=test_id, + jira_issue_key="TST-11", + jira_issue_id="10011", + created_by=admin_user.id, + ) + db.add(link) + admin_user.tempo_api_token = None # relying on the admin token, not a personal one + admin_user.jira_account_id = "jira-account-123" + db.commit() + + mock_tempo_instance = MagicMock() + mock_tempo_instance.create_worklog.return_value = {"id": "wl-2"} + mock_tempo_cls.return_value = mock_tempo_instance + + test = MagicMock() + test.id = test_id + test.name = "Phishing simulation" + test.red_started_at = datetime(2026, 5, 18, 10, 0, 0) + test.updated_at = datetime(2026, 5, 18, 12, 0, 0) + test.created_at = test.updated_at + + result = tempo_service.auto_log_test_worklog(db, test, admin_user, "red_team_execution") + + assert result == {"id": "wl-2"} + mock_tempo_instance.create_worklog.assert_called_once() + + def test_get_tempo_client_raises_when_disabled(monkeypatch): monkeypatch.setattr("app.services.tempo_service.settings.TEMPO_ENABLED", False) with pytest.raises(InvalidOperationError, match="not enabled"): diff --git a/frontend/src/index.css b/frontend/src/index.css index f1d8c73..b510798 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -1 +1,8 @@ @import "tailwindcss"; + +/* App is dark-themed throughout — tell the browser so native controls + (date/time picker popups, scrollbars, etc.) render dark instead of + the default light/white UI. */ +:root { + color-scheme: dark; +} diff --git a/frontend/src/pages/TestDetailPage.tsx b/frontend/src/pages/TestDetailPage.tsx index 81a56c0..295ca30 100644 --- a/frontend/src/pages/TestDetailPage.tsx +++ b/frontend/src/pages/TestDetailPage.tsx @@ -1,7 +1,7 @@ import { useParams, useNavigate } from "react-router-dom"; import MarkdownText from "../components/MarkdownText"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; import { Loader2, AlertCircle, ArrowLeft, BookOpen, X, CheckCircle } from "lucide-react"; import { @@ -143,9 +143,15 @@ export default function TestDetailPage() { staleTime: 5 * 60 * 1000, }); - // Hydrate drafts from test data + // Hydrate drafts from test data — only once per test, on first load or + // when navigating to a different test. Refetches triggered by evidence + // uploads, timer ticks, etc. must NOT re-run this, or they'd stomp + // whatever the user is still typing (e.g. execution_start_time) with + // whatever is still saved server-side. + const hydratedTestId = useRef(null); useEffect(() => { - if (test) { + if (test && hydratedTestId.current !== test.id) { + hydratedTestId.current = test.id; setRedDraft({ procedure_text: test.procedure_text || "", tool_used: test.tool_used || "",