feat(m7): blue review fields + spec amendment + reviewer follow-ups
User feedback after the M7 ship: blue team's Excel workflow had 5 extra
fields we didn't capture. Per-test page also doesn't match their
workflow — they need a tabular view, one table per scenario.
Spec
- tasks/spec.md amended (`revised: 2026-05-15`): §4 in-scope, §F6, §8
model bullet. §F6 now pins the column matrix, single-row-edit
semantics, Esc-cancel, blur-confirm, and reconciles detection_level
as a pill inside the Commentaires cell (no 8th column).
- tasks/todo.md M7 section grew an "Amendement 2026-05-15" sub-block
tracking backend ☑ and frontend ☐.
Backend
- Migration c2a8f4b1d6e9: 5 nullable columns on mission_tests
(blue_log_source, blue_siem_logs, blue_incident_at,
blue_incident_number, blue_incident_recipient_email).
- _BLUE_FIELDS extended; update_mission_test_fields propagates each
field; MissionTestDetailView + MissionTestView (the nested view in
GET /missions/{id}) surface every annotation field, plus
last_actor_*, updated_at, detection_level_key — O(1) batch lookup
for detection-level keys and last-actor users keeps it scalable.
- UpdateMissionTestPayload accepts each field with length caps
(120/200_000/120/255).
Reviewer follow-ups applied
- blue_incident_at + executed_at now reject naïve datetimes
(_ensure_aware_datetime) — Postgres would otherwise interpret
them in the session TZ, defeating the M7 verbatim-time contract.
- blue_incident_recipient_email goes through a permissive RFC-shape
regex (_validate_email_shape) so internal/lab TLDs like .local
/ .corp / .test pass — Pydantic EmailStr is too strict (lessons.md
M2 trap).
- Project-wide: switched `e.errors()` to
`e.errors(include_context=False, include_url=False)` because the
AfterValidator-raised ValueError lands in ctx and Flask can't
serialize it.
Tests
- 5 new pytest cases: blue user writes the 5 new fields, red user is
individually 403'd on each, round-trip via GET, naïve datetime
rejected, email shape validated (.local accepted, bad shape 400).
- 138 pytest green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -165,6 +165,12 @@ class MissionTestDetailView:
|
||||
blue_comment_md: str | None
|
||||
detection_level_id: uuid.UUID | None
|
||||
detection_level_key: str | None
|
||||
# Post-M7 blue review fields (cf. user feedback 2026-05-15).
|
||||
blue_log_source: str | None
|
||||
blue_siem_logs: str | None
|
||||
blue_incident_at: datetime | None
|
||||
blue_incident_number: str | None
|
||||
blue_incident_recipient_email: str | None
|
||||
last_actor_id: uuid.UUID | None
|
||||
last_actor_email: str | None
|
||||
last_actor_display_name: str | None
|
||||
@@ -348,6 +354,11 @@ def _to_detail_view(
|
||||
blue_comment_md=test.blue_comment_md,
|
||||
detection_level_id=test.detection_level_id,
|
||||
detection_level_key=level_key,
|
||||
blue_log_source=test.blue_log_source,
|
||||
blue_siem_logs=test.blue_siem_logs,
|
||||
blue_incident_at=test.blue_incident_at,
|
||||
blue_incident_number=test.blue_incident_number,
|
||||
blue_incident_recipient_email=test.blue_incident_recipient_email,
|
||||
last_actor_id=test.last_actor_id,
|
||||
last_actor_email=last_actor_email,
|
||||
last_actor_display_name=last_actor_display_name,
|
||||
@@ -448,7 +459,15 @@ def list_activity_since(
|
||||
# Side membership for each writable field (mirror of the spec's red/blue split).
|
||||
_RED_FIELDS = {"red_command", "red_output", "red_comment_md",
|
||||
"executed_at", "executed_at_overridden"}
|
||||
_BLUE_FIELDS = {"blue_comment_md", "detection_level_id"}
|
||||
_BLUE_FIELDS = {
|
||||
"blue_comment_md",
|
||||
"detection_level_id",
|
||||
"blue_log_source",
|
||||
"blue_siem_logs",
|
||||
"blue_incident_at",
|
||||
"blue_incident_number",
|
||||
"blue_incident_recipient_email",
|
||||
}
|
||||
|
||||
|
||||
def _classify_fields(touched: set[str]) -> tuple[bool, bool]:
|
||||
@@ -474,6 +493,11 @@ def update_mission_test_fields(
|
||||
detection_level_id: Any = _UNSET,
|
||||
executed_at: Any = _UNSET,
|
||||
executed_at_overridden: Any = _UNSET,
|
||||
blue_log_source: Any = _UNSET,
|
||||
blue_siem_logs: Any = _UNSET,
|
||||
blue_incident_at: Any = _UNSET,
|
||||
blue_incident_number: Any = _UNSET,
|
||||
blue_incident_recipient_email: Any = _UNSET,
|
||||
) -> MissionTestDetailView:
|
||||
"""Patch any subset of the red/blue annotation fields.
|
||||
|
||||
@@ -495,6 +519,16 @@ def update_mission_test_fields(
|
||||
touched.add("executed_at")
|
||||
if executed_at_overridden is not _UNSET:
|
||||
touched.add("executed_at_overridden")
|
||||
if blue_log_source is not _UNSET:
|
||||
touched.add("blue_log_source")
|
||||
if blue_siem_logs is not _UNSET:
|
||||
touched.add("blue_siem_logs")
|
||||
if blue_incident_at is not _UNSET:
|
||||
touched.add("blue_incident_at")
|
||||
if blue_incident_number is not _UNSET:
|
||||
touched.add("blue_incident_number")
|
||||
if blue_incident_recipient_email is not _UNSET:
|
||||
touched.add("blue_incident_recipient_email")
|
||||
|
||||
needs_red, needs_blue = _classify_fields(touched)
|
||||
if not viewer_is_admin:
|
||||
@@ -534,6 +568,27 @@ def update_mission_test_fields(
|
||||
raise InvalidTestPayload("unknown detection_level_id")
|
||||
test.detection_level_id = detection_level_id
|
||||
|
||||
# Post-M7 blue-side review fields — short text + long text + a small
|
||||
# cyber-incident sub-record. Email is sanity-checked at the API layer
|
||||
# via Pydantic; the service just normalises empty strings to NULL.
|
||||
if "blue_log_source" in touched:
|
||||
test.blue_log_source = _opt_md(blue_log_source)
|
||||
if "blue_siem_logs" in touched:
|
||||
# SIEM excerpts can legitimately have leading whitespace inside
|
||||
# the body (table-like log lines), so use the command-style
|
||||
# normaliser that only collapses purely-empty strings to NULL.
|
||||
test.blue_siem_logs = _opt_cmd(blue_siem_logs)
|
||||
if "blue_incident_at" in touched:
|
||||
if blue_incident_at is not None and not isinstance(blue_incident_at, datetime):
|
||||
raise InvalidTestPayload("blue_incident_at must be an ISO datetime")
|
||||
test.blue_incident_at = blue_incident_at
|
||||
if "blue_incident_number" in touched:
|
||||
test.blue_incident_number = _opt_md(blue_incident_number)
|
||||
if "blue_incident_recipient_email" in touched:
|
||||
test.blue_incident_recipient_email = _opt_md(
|
||||
blue_incident_recipient_email
|
||||
)
|
||||
|
||||
if "executed_at_overridden" in touched or "executed_at" in touched:
|
||||
# Editing executed_at is a red-only privilege and only valid when
|
||||
# the test is past the `executed` milestone. Spec M7: override is
|
||||
|
||||
@@ -129,6 +129,24 @@ class MissionTestView:
|
||||
executed_at_overridden: bool
|
||||
mitre_tags: list[MissionMitreTagView]
|
||||
source_test_template_id: uuid.UUID | None
|
||||
# Annotation fields are surfaced here so the mission detail page can
|
||||
# render the full scenario table without a per-test round trip (the
|
||||
# batch lookups for detection_level_key + last_actor below stay O(1)).
|
||||
red_command: str | None
|
||||
red_output: str | None
|
||||
red_comment_md: str | None
|
||||
blue_comment_md: str | None
|
||||
detection_level_id: uuid.UUID | None
|
||||
detection_level_key: str | None
|
||||
blue_log_source: str | None
|
||||
blue_siem_logs: str | None
|
||||
blue_incident_at: datetime | None
|
||||
blue_incident_number: str | None
|
||||
blue_incident_recipient_email: str | None
|
||||
last_actor_id: uuid.UUID | None
|
||||
last_actor_email: str | None
|
||||
last_actor_display_name: str | None
|
||||
updated_at: datetime
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -530,14 +548,60 @@ def _member_views(s: Session, mission: Mission) -> list[MissionMemberView]:
|
||||
return out
|
||||
|
||||
|
||||
def _scenario_views(scenarios: list[MissionScenario]) -> list[MissionScenarioView]:
|
||||
def _scenario_views(
|
||||
s: Session, scenarios: list[MissionScenario]
|
||||
) -> list[MissionScenarioView]:
|
||||
"""Assemble scenario views. `mission_scenarios` and `mission_tests` both
|
||||
carry `SoftDeleteMixin`; M6 doesn't surface soft-deletion of those rows in
|
||||
any endpoint, but the filter is applied here so future deletions (M7+)
|
||||
don't drift the rendered list silently."""
|
||||
don't drift the rendered list silently.
|
||||
|
||||
The annotation fields (red/blue review state) are surfaced too so the
|
||||
front-end scenario table renders in a single GET. To keep the call O(1)
|
||||
in the number of tests, the detection-level keys and last-actor labels
|
||||
are batch-loaded.
|
||||
"""
|
||||
from app.models.auth import User as _User # noqa: PLC0415 — local import to avoid a cycle
|
||||
from app.models.setting import DetectionLevel as _DetectionLevel # noqa: PLC0415
|
||||
|
||||
live_scenarios = [sc for sc in scenarios if sc.deleted_at is None]
|
||||
if not live_scenarios:
|
||||
return []
|
||||
|
||||
# Collect every (test) annotation FK upfront so we can batch the two
|
||||
# extra queries (detection levels + last-actor users) instead of doing
|
||||
# one s.get() per row.
|
||||
level_ids: set[uuid.UUID] = set()
|
||||
actor_ids: set[uuid.UUID] = set()
|
||||
for sc in live_scenarios:
|
||||
for t in sc.tests:
|
||||
if t.deleted_at is not None:
|
||||
continue
|
||||
if t.detection_level_id is not None:
|
||||
level_ids.add(t.detection_level_id)
|
||||
if t.last_actor_id is not None:
|
||||
actor_ids.add(t.last_actor_id)
|
||||
|
||||
level_keys: dict[uuid.UUID, str] = {}
|
||||
if level_ids:
|
||||
for row in s.execute(
|
||||
select(_DetectionLevel.id, _DetectionLevel.key).where(
|
||||
_DetectionLevel.id.in_(level_ids)
|
||||
)
|
||||
).all():
|
||||
level_keys[row.id] = row.key
|
||||
|
||||
actors: dict[uuid.UUID, tuple[str, str | None]] = {}
|
||||
if actor_ids:
|
||||
for row in s.execute(
|
||||
select(_User.id, _User.email, _User.display_name).where(
|
||||
_User.id.in_(actor_ids)
|
||||
)
|
||||
).all():
|
||||
actors[row.id] = (row.email, row.display_name)
|
||||
|
||||
views: list[MissionScenarioView] = []
|
||||
live = [sc for sc in scenarios if sc.deleted_at is None]
|
||||
for sc in sorted(live, key=lambda s_: s_.position):
|
||||
for sc in sorted(live_scenarios, key=lambda s_: s_.position):
|
||||
test_views: list[MissionTestView] = []
|
||||
live_tests = [t for t in sc.tests if t.deleted_at is None]
|
||||
for t in sorted(live_tests, key=lambda t_: t_.position):
|
||||
@@ -552,6 +616,10 @@ def _scenario_views(scenarios: list[MissionScenario]) -> list[MissionScenarioVie
|
||||
t.mitre_tags, key=lambda tg: (tg.mitre_kind, tg.mitre_external_id)
|
||||
)
|
||||
]
|
||||
actor_email: str | None = None
|
||||
actor_display: str | None = None
|
||||
if t.last_actor_id is not None and t.last_actor_id in actors:
|
||||
actor_email, actor_display = actors[t.last_actor_id]
|
||||
test_views.append(
|
||||
MissionTestView(
|
||||
id=t.id,
|
||||
@@ -571,6 +639,25 @@ def _scenario_views(scenarios: list[MissionScenario]) -> list[MissionScenarioVie
|
||||
executed_at_overridden=t.executed_at_overridden,
|
||||
mitre_tags=tag_views,
|
||||
source_test_template_id=t.source_test_template_id,
|
||||
red_command=t.red_command,
|
||||
red_output=t.red_output,
|
||||
red_comment_md=t.red_comment_md,
|
||||
blue_comment_md=t.blue_comment_md,
|
||||
detection_level_id=t.detection_level_id,
|
||||
detection_level_key=(
|
||||
level_keys.get(t.detection_level_id)
|
||||
if t.detection_level_id
|
||||
else None
|
||||
),
|
||||
blue_log_source=t.blue_log_source,
|
||||
blue_siem_logs=t.blue_siem_logs,
|
||||
blue_incident_at=t.blue_incident_at,
|
||||
blue_incident_number=t.blue_incident_number,
|
||||
blue_incident_recipient_email=t.blue_incident_recipient_email,
|
||||
last_actor_id=t.last_actor_id,
|
||||
last_actor_email=actor_email,
|
||||
last_actor_display_name=actor_display,
|
||||
updated_at=t.updated_at,
|
||||
)
|
||||
)
|
||||
views.append(
|
||||
@@ -589,7 +676,7 @@ def _scenario_views(scenarios: list[MissionScenario]) -> list[MissionScenarioVie
|
||||
def _to_detail_view(s: Session, m: Mission) -> MissionView:
|
||||
scenarios = [sc for sc in m.scenarios if sc.deleted_at is None]
|
||||
members = _member_views(s, m)
|
||||
scenario_views = _scenario_views(scenarios)
|
||||
scenario_views = _scenario_views(s, scenarios)
|
||||
tests_count = sum(len(sc.tests) for sc in scenario_views)
|
||||
return MissionView(
|
||||
id=m.id,
|
||||
|
||||
Reference in New Issue
Block a user