Files
Metamorph/tasks/lessons.md
Knacky ed70458d8f feat(m7): per-test execution — red/blue zones, evidence pipeline, activity poll
DoD M7 (spec §F5 + §F6 + §F8 + tasks/todo.md M7) covered end-to-end:

Backend
- New migration `91a4e7c6d2f3` adds `mission_tests.last_actor_id` (FK users
  ON DELETE SET NULL) and `ix_mission_tests_updated_at` for the polling query.
- `detection_levels`: 4 default rows seeded at boot, `GET /detection-levels`
  read-only (CRUD lands in M8).
- `mission_tests` service + `missions` API extension:
  - `GET /missions/{id}/tests/{test_id}` — full detail incl. evidence list
  - `PUT  /missions/{id}/tests/{test_id}` — patch red/blue fields with per-field
    perm classification (`mission.write_red_fields` vs `mission.write_blue_fields`)
  - `POST /missions/{id}/tests/{test_id}/transition` — pending↔skipped/blocked
    and pending→executed→reviewed_by_blue (+ undo paths), side-aware perm gate
    that fires *before* idempotency, `executed_at` auto-stamped on the way in
  - `GET  /missions/{id}/activity?since=<ISO>` — drives the 15 s polling badge
- `evidence` service + top-level `/evidence/<id>` API:
  - Streaming upload, SHA256 chunk-by-chunk, 25 MB cap, ext+MIME whitelist
  - Content-addressed storage at ${EVIDENCE_DIR}/<mission>/<test>/<sha256><ext>
  - Atomic `os.replace`, hex-validated SHA path component, root-dir guard
  - Membership-aware (404 on miss/forbidden, no existence leak)
- `/diag/reset` now wipes ${EVIDENCE_DIR}/* in test mode (symlink-safe) and
  re-seeds detection levels as a safety net.

Frontend
- `lib/missions.ts` — M7 types + queryKey factory + state-machine matrix.
- `pages/MissionTestPage.tsx` — two-zone layout: red border (command, output,
  comment, mark-executed + override toggle) and cyan border (detection-level
  select, comment, drag-and-drop evidence dropzone). Last-touched badge polls
  /activity every 15 s, gated on document.visibilityState. Per-field disable
  based on the user's red/blue perms (server stays the arbiter).
- `pages/MissionDetailPage.tsx` — test rows link to the new per-test page.
- `App.tsx` — registers /missions/:id/tests/:testId behind RequireAuth.
- `HomePage.tsx` — hero + roadmap card bumped to M7; next is M8.

Tests
- `backend/tests/test_mission_tests.py` — 27 pytest tests (red/blue field
  gating, state-machine matrix incl. idempotent-side enforcement, executed_at
  override, 24/26 MB upload + SHA256, MIME/ext whitelist, soft-delete hide,
  activity polling with URL-encoded `since`, membership 404 vs admin bypass,
  cross-mission evidence access).
- `e2e/tests/m7-execution.spec.ts` — 5 Playwright tests against the live stack
  (red-only/blue-only API gating, mark-executed + reviewed_by_blue side
  enforcement, 24 MB/26 MB upload + SHA256 round-trip, SPA per-test page save
  + transition, non-member 404 message). afterAll restores stable admin and
  re-syncs MITRE.

Docs
- CHANGELOG.md: M7 section + post-M7 review-pass subsection.
- README.md: status, feature blurb, roadmap, testing-m7 link.
- tasks/testing-m7.md: manual + automated procedure with transition matrix
  and perm-gating table.
- tasks/lessons.md: M7 retrospectives (LogRecord `created` trap, URL-encoded
  query timestamps, perm-before-flush, atomic move, polling visibility gate).

Test count: 133 pytest / 49 Playwright, all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 08:16:48 +02:00

124 lines
28 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
type: lessons
project: Metamorph
---
# Metamorph — Lessons learned
> Capture session-level retrospectives here: surprises, traps avoided, decisions revisited. Keep entries short and actionable. Most recent first.
## 2026-05-08 — M0 bootstrap
- Spec finalisée d'abord (`tasks/spec.md`), 8 tours de questions ciblées avant tout code → 0 hypothèse latente avant M0. Pattern à reproduire pour les futurs projets greenfield.
- Choix `uv` pour le backend Python (rapidité de lock, image Docker plus mince qu'avec poetry).
- TLS terminé par reverse proxy externe (cf. spec §6 NF-network) → pas de Caddy/Traefik dans le compose, simplifie le M0.
- Bootstrap du 1er admin via token affiché dans les logs : retenu sur Token-in-logs plutôt que ENV pour éviter de mettre le password en clair dans `.env`.
- **Piège Dockerfile** : la process-substitution bash `<(...)` ne marche pas dans une instruction `RUN` Docker car le shell par défaut est `sh`, pas `bash`. Soit ajouter `SHELL ["/bin/bash", "-c"]`, soit refactor sans process-sub. Ici j'ai préféré refactor (plus portable) : `uv venv` + `uv pip install --python /opt/venv/bin/python .`. Quand un `uv.lock` existera, basculer sur `uv sync --frozen --no-dev`.
- Vérification d'un compose sans Docker installé : `python3 -c "import yaml; yaml.safe_load(open('docker-compose.yml'))"` valide la syntaxe YAML, et un script qui croise les `environment:` du compose avec `.env.example` détecte les variables manquantes côté docs.
- **Lancer le subagent `spec-reviewer` à chaque fin de milestone** (HARD RULE 4 du CLAUDE.md global). J'avais oublié à la fin de M0 ; le user me l'a rappelé. Le reviewer a remonté 6 défauts légitimes en quelques minutes (pre-commit absent, fonts via CDN, secrets par défaut non gardés, `make dev` no-op, `database_url` dead-code, Node engines non pinned). À automatiser dans le workflow de fin de milestone.
- **Spec §7 "pas de CDN runtime"** s'applique aussi aux fonts, pas seulement aux libs JS. Self-host via `@fontsource/<name>` plutôt que Google Fonts `<link>` — bonus OPSEC (pas de fingerprinting via fonts.googleapis.com).
- **Pattern de garde de secrets** : un `model_validator` Pydantic qui refuse de booter en `APP_ENV != "dev"` avec des secrets manquants ou égaux aux placeholders de `.env.example`. Coût quasi nul, élimine la classe entière des "oubli de set en prod".
- **Makefile portable docker/podman** : `ENGINE := $(shell command -v docker … podman …)`, puis sélection du compose driver en fonction (`docker compose` vs `podman compose` vs `podman-compose` legacy). Le piège classique `COMPOSE ?=` ne marche pas si on veut conditionner la valeur par défaut sur `ENGINE` — il faut `ifndef COMPOSE` + `ifeq ($(ENGINE),docker)`. Tous les targets restent compose-driven (`$(COMPOSE) exec`, etc.) ; seuls `volumes` / `inspect-health` / `logs-api` ont besoin de `$(ENGINE)` directement, et même là on évite les filtres par label projet (instables entre podman-compose et docker compose) en se reposant sur `container_name:` du compose file.
## 2026-05-10 — M0 DoD validation (réelle, pas paperware)
- **JE DOIS LANCER LE DoD MOI-MÊME avant de déclarer un milestone done.** L'utilisateur me l'a fait remonter ; le `make up` initial échouait sur 3 problèmes que la revue statique n'a pas vus. Règle : à chaque fin de milestone, exécuter le DoD localement (`make up` + smoke + e2e) en plus du spec-reviewer.
- **Podman + Fedora exige des FQDN d'image** (`docker.io/library/postgres:16-alpine`, pas `postgres:16-alpine`). Le mode `short-name-mode=enforcing` fail sans TTY pour prompter. Docker accepte le même préfixe transparente. → Dorénavant tous les `image:` et `FROM …` des projets cross-engine sont qualifiés.
- **`.dockerignore` qui exclut `*.md` casse `pyproject.toml` qui référence `readme = "README.md"`** : hatchling lit le README au build pour valider les métadonnées. Soit on copie le README explicitement, soit on n'exclut pas les `*.md`, soit on retire la clé `readme`. J'ai retiré la clé pour découpler.
- **`extends HTMLAttributes<HTMLDivElement>` clash sur `title`** : la prop native est `string`, donc redéfinir `title?: ReactNode` produit TS2430. Pattern à retenir : `Omit<HTMLAttributes<…>, 'title'>` quand on overload `title`/`color`/`autoFocus` etc.
- **Podman-compose 1.x ne surfait pas les `HEALTHCHECK` du Dockerfile dans `podman inspect`** : il faut redéclarer le healthcheck dans le `docker-compose.yml` pour que `make inspect-health` voie réellement l'état. Bonus : c'est aussi plus portable.
- **Piège shell : `make up 2>&1 | tail -80` bloque** quand la sortie est petite, parce que `tail` bufferise jusqu'à recevoir SIGPIPE en fin de pipeline ; quand le build est lent, on n'a aucune sortie pendant des minutes. Fix : rediriger vers fichier (`>/tmp/log 2>&1`) puis `tail` séparément, ou utiliser le `Monitor` tool pour streamer.
- **`PODMAN_COMPOSE_WARNING_LOGS=false`** masque le banner "Executing external compose provider …" qui spamme chaque commande. À exporter depuis le Makefile.
## 2026-05-10 — M1 schéma DB & migrations
- **Compose pioche le DERNIER stage du Dockerfile par défaut.** En ajoutant un stage `test` après `runtime`, le container `api` s'est mis à exécuter `python -m pytest` au lieu de `gunicorn`, en boucle (exit 1 → restart → exit 1). Fix : `target: runtime` explicite dans `docker-compose.yml`. Règle : **toujours préciser `target:` quand un Dockerfile a >1 stage final viable.**
- **Snapshot vs référence (spec §11)** : pour qu'un snapshot survive à un re-sync de la référence (ex : MITRE qui retire une technique), il faut **dénormaliser les champs descriptifs** dans la table snapshot (ici `mitre_external_id`, `mitre_name`, `mitre_url`) et **ne pas mettre de FK** vers la table source. Si on garde une FK, la cascade détruit la donnée historique (CASCADE) ou bloque le sync (RESTRICT). La dénormalisation est le bon trade-off pour un état figé en lecture après archivage.
- **`SoftDeleteMixin.__table_args__` est silencieusement écrasé** par la classe enfant qui déclare son propre `__table_args__`. Pattern à éviter pour les mixins qui veulent ajouter des contraintes/index. Soit ne rien mettre dans `__table_args__` du mixin (et imposer aux classes de déclarer l'index), soit utiliser `event.listens_for("after_parent_attach", ...)`. J'ai choisi la 1re option : explicite > magique.
- **Workflow Alembic en container** : `alembic revision --autogenerate` crée le fichier dans le container, qu'il faut `podman cp` vers l'host avant rebuild. Sinon perdu. Ajouter ce détail dans la doc M1 (et envisager un bind mount `dev` plus tard).
- **Bypass `APP_ENV` doit couvrir `dev` ET `test`** : un container test légitime ne doit pas avoir besoin de secrets prod-grade. `if self.APP_ENV in ("dev", "test"): return self`.
- **`pytest` dans le runtime image, c'est non.** Faire un stage `test` dédié (multi-stage `--target test`) qui étend `deps` + `dev extras` + `tests/`, lancé via `podman run --rm --network <project>_<network>` en éphémère. Le runtime reste minimal en prod.
- **Le test d'intégration "expected tables/FK/CHECK" est le bon filet de sécurité** pour M1+ : il a immédiatement attrapé les fixes du reviewer (le retrait de `ck_mission_test_mitre_tags_exactly_one_mitre_fk` aurait été un oubli silencieux sinon).
- **Lancer le DoD avant de dire "M1 done"** : règle gravée à M0, respectée ici. `make clean && make up && make migrate && make test-api && make e2e` est la séquence canonique de fin de milestone.
## 2026-05-12 — M4 MITRE ATT&CK
- **STIX parsing avec stdlib uniquement** (`urllib.request` + `json` + `hashlib`) suffit pour 50 MB de bundle, ~1.1 s parse end-to-end. Pas besoin de `requests`/`httpx`. Toute future ingestion de gros JSON pinné → stdlib first, ne pas inflater l'image pour un cas d'usage one-shot.
- **Le sous-jacent MITRE évolue** : la spec mentionne "14 tactics" mais la v19 actuelle en ship 15 (Reconnaissance + Resource Development depuis v8). Les assertions de DoD sont à exprimer en `>= X` quand un référentiel externe est en jeu, pas en `== X`. Pattern : décorréler la valeur exacte du contrat (sinon la maintenance casse au prochain bump).
- **Sub-technique parent resolution** : la source authoritative est la `relationship[subtechnique-of]` STIX, pas la convention dotted-id `T1003.001 → T1003`. La regex en fallback ne sert que si la relation manque (jamais le cas avec MITRE officiel, mais utile pour bundles custom).
- **`session_scope()` enveloppe tout le seed dans une seule transaction** → les lecteurs externes ne voient jamais un état intermédiaire pendant le DELETE+INSERT de `mitre_technique_tactics`. Postgres READ COMMITTED isole. Pas besoin d'advisory lock sauf si on s'attend à des syncs concurrents.
- **Checksum bypass silencieux = footgun**. Avant : `source != MITRE_DEFAULT_URL``expected_sha256 = None`. Un admin qui type un domaine attaquant dans `mitre_source_url` ingère du JSON arbitraire sans intégrité. Patron correct : `MitreSeedError("custom URL requires an expected_sha256 or allow_unverified=True")`. L'opt-out explicite (`--skip-checksum` côté CLI, `allow_unverified: true` côté API) reste possible mais visible.
- **`/diag/reset` cohérence** : si on TRUNCATE `settings` mais pas les tables de référence MITRE (gardées car coûteuses à re-seeder), `GET /mitre/status` retourne `last_sync: null` alors que `GET /mitre/tactics` retourne 15 lignes. Discrepancy mensongère. Fix : TRUNCATE aussi les `mitre_*` dans `/diag/reset` (test-only endpoint, on accepte la re-sync via `/mitre/sync` en `beforeAll`).
- **Volume permissions et chown au build** : `mkdir -p /data/mitre && chown -R metamorph:metamorph /data` dans le Dockerfile suffit POUR le premier `make up` (podman copie l'ownership de l'image lors de l'init du named volume). Mais si un volume préexiste owned root, le chown ne replay pas. À documenter en pré-requis dans `tasks/testing-m<N>.md`, ou ajouter un entrypoint shim qui valide les perms au boot.
- **Build cache du front silencieux** : `podman build` montre `Using cache aad724...` même quand src/ a changé si le diff entre les arborescences est invisible (mtime). En cas de doute : `podman build --no-cache` une fois pour confirmer que le typecheck passe, puis `make down && make up` pour pousser le bundle. Réflexe à garder en mémoire.
## 2026-05-11 — M3 RBAC, groupes, users, invitations
- **`logging.LogRecord` réserve `name`** comme attribut interne (en plus de `message`, `levelname`, `pathname`, `filename`, `module`, `funcName`, `lineno`, `asctime`, `process`, `thread`, `args`). Donc `log.info("metamorph.x.created", extra={"name": entity.name})` lève `KeyError: "Attempt to overwrite 'name' in LogRecord"`. Patron : préfixer toute clé risquée par l'entité (`group_name`, `user_name`, `template_name`). À documenter dans le style guide quand on en aura un.
- **Pattern "sentinel pour distinguer absent vs null"** : Pydantic ne sait pas distinguer `{}` de `{"display_name": null}` quand le champ est `str | None = None`. Solution : lire `raw = request.get_json()` puis tester `"display_name" in raw` dans la couche API, passer un sentinel `...` au service, qui distingue "ne pas toucher" de "set à None". Lourd mais explicite. Si ça revient souvent, encapsuler dans un helper `triState(raw, key, payload)`.
- **`limiter.reset()` flask-limiter** est public et clean — pas besoin de toucher à `limiter._storage`. À appeler dans `/diag/reset` quand le limiter est `enabled`. Toujours guarder avec `if limiter.enabled` pour ne pas planter en `APP_ENV=test`.
- **Rate-limit scope `APP_ENV in ("prod", "staging")`** : meilleure granularité que prod-only. La spec NF-security est *operator-facing*, pas dev. Trade-off réconcilié dans `app/core/rate_limit.py` avec un docstring explicite. Dev = ergonomics totale, prod/staging = limiter actif, test = désactivé.
- **Playwright `workers: 1` + `fullyParallel: false`** quand chaque spec file fait du `/diag/reset` (DB partagée). Avec parallélisme, les workers se truncate mutuellement entre eux → install token consumé, etc. Pattern simple et robuste : un seul worker pour les e2e, parallélisme intra-file laissé à `test.describe.configure({ mode: 'serial' })`.
- **Sessions Playwright entre tests** : chaque `test()` reçoit une `page` neuve (BrowserContext fresh). Pas de partage de session entre tests du même `describe`. Helper `loginViaSpa()` à appeler au début de chaque test SPA-driven (les tests purement API peuvent partager via une variable de spec mais c'est rare). Alternative : `storageState` global, mais ça complique le truncate workflow.
- **Dual seed = boot + bootstrap** : seeder les perms au boot ET dans `bootstrap_admin()` n'est pas redondant. Sur DB fraîchement migrée vide, le boot suffit. Mais après `/diag/reset` (qui TRUNCATE `permissions` + `group_permissions` + `groups`), seul `/setup` re-déclenche le chemin de seed via `bootstrap_admin → seed_all`. Sans ce 2e appel, l'admin créé aurait `is_admin=True` mais le catalogue serait vide.
- **Snapshot UserView/GroupView détachés** : retourner des `@dataclass(frozen=True)` au lieu de l'ORM permet de fermer le `session_scope` immédiatement. Plus simple que `s.expunge()` pour chaque champ, et la couche API peut sérialiser sans lazy-loading. Patron à reproduire pour tous les services.
- **Invariant "admin a toutes les perms"** : même si le décorateur bypass via `is_admin = "admin" in group_names` (et pas via le perm set), garder l'invariant côté API en refusant `set_group_permissions(admin_group, !=all_codes)`. Future-proof : si on bouge le bypass à un check perm-based plus tard, l'invariant tient déjà. `SystemGroupProtected` réutilisé pour le 409.
- **Toujours rebuild front + recreate containers** : `make rebuild` ne recrée pas les containers, donc le bundle nginx reste l'ancien. Patron canonique : `make down && make up`. Documenté pour la 2e fois dans M3 ; à faire passer en runbook au prochain `tasks/testing-m<N>.md`.
## 2026-05-10 — M2 auth, JWT, invitations
- **`pydantic.EmailStr` rejette les TLD réservés** (`.local`, `.corp`, `.test`, …) via `email-validator` `globally_deliverable=True`. Pour un outil red-team utilisé en lab/intranet, créer un type custom permissif (`Annotated[str, AfterValidator(...)]`) avec une regex RFC-shape. À garder en tête pour tout futur projet "internal".
- **Cookies `Secure=True` sur `localhost` HTTP** : modern browsers (Chrome ≥89, Firefox ≥75) traitent `localhost` comme un secure context et acceptent les cookies `Secure` même servis en HTTP. Donc on peut respecter la spec strictement (`Secure` toujours) sans casser le dev — pas besoin de gating par `APP_ENV`.
- **`getByLabel` de Playwright** prend le **nom accessible** de l'input. Quand un `<label>` enveloppe `input` + `<span>` hint + `<span>` error, le hint et l'error polluent le nom et `getByLabel('Password', exact: true)` ne matche plus. Pattern correct : `<div>` parent, `<label htmlFor>` séparé du `<input id>`, hint et error en `<p>` siblings hors du `<label>`.
- **`flask-limiter` doit être désactivé en `APP_ENV=test`** sinon les tests qui font 10+ logins de suite rate-limit. `Limiter(..., enabled=settings.APP_ENV != "test")` règle le cas globalement.
- **`pydantic[email]` extra** est REQUIS dès qu'on utilise `EmailStr`. Ne pas s'en rendre compte donne un crash gunicorn worker au boot avec `ImportError: email-validator is not installed`. À dupliquer dans le starter pyproject pour les futurs projets.
- **Compose `target:` est OBLIGATOIRE** quand un Dockerfile a un stage après le runtime — par défaut compose builde le DERNIER stage. J'ai été mordu deux fois (M1 puis M2). Désormais : tout Dockerfile multi-stage avec un stage de test/dev → `target: runtime` explicite dans `docker-compose.yml`.
- **Refresh token rotation + chain revoke** : à chaque `/auth/refresh`, on marque l'ancien token `revoked_at` + `replaced_by_id`. Si quelqu'un re-présente un token déjà rotaté, on cascade-revoke toute la chaîne (compromise probable). Pattern à reproduire pour tout système JWT à long terme.
- **`make rebuild` ne recrée pas les containers** — il faut `make down && make up` après un changement front pour que nginx serve le nouveau bundle. Important quand on debug un test e2e qui attend un selecteur récemment ajouté côté React.
- **`podman compose stop api` puis `up -d api` casse les dépendances** entre containers (`db` healthy → `api` depends on it) : podman-compose ne résout pas la chaîne de deps quand on cible un seul service. Pour un override d'env, mieux vaut `make down && APP_ENV=test make up`.
- **`/diag/reset` test-only** : exposer un endpoint qui truncate la DB est tentant pour les e2e mais ouvre une grosse surface en cas de fuite. Compromise actuel : autorisé en `dev` ET `test` (pas en prod), avec un log `WARNING` à chaque appel. Si jamais on déploie une stack dev publique, **désactiver** l'endpoint via env var.
## 2026-05-14 — M7 execution + evidence + activity
- **`logging.LogRecord` reserves `created`** — same trap as `name` (M3 lessons): `extra={"created": n}` raises `KeyError: "Attempt to overwrite 'created' in LogRecord"`. Pattern: prefix with the entity (`rows_created`). The `created` is the LogRecord timestamp, hence the conflict. Reserved-key cheatsheet (kept growing): `name, msg, args, levelname, levelno, pathname, filename, module, funcName, created, msecs, lineno, thread, threadName, process`.
- **Query string `+` is `%20` once `request.args` decodes it.** A naked ISO datetime in `?since=2026-05-14T07:55:16+00:00` arrives as `2026-05-14T07:55:16 00:00`, which `datetime.fromisoformat` rejects with `ValueError`. The fix is on the *client* (URL-encode) — not on the server (a tolerant "space → +" reparse would conflate real-spaces with un-encoded plusses). Now codified in `testing-m7.md` §7 + every test that hits `/activity?since=` calls `urllib.parse.quote`.
- **Field-level perm enforcement must happen *before* the SQL transaction.** First M7 draft did `_load_test(...)` then `if not allowed: raise`. Two issues: (a) extra DB hit on a refused request, (b) audit log conflated "row exists" with "perm denied". Refactor: classify the touched fields → check perms → only then enter `session_scope`. Cleaner audit log and one fewer round-trip on the 403 path.
- **Streamed upload + atomic move is the canonical pattern for content-addressed evidence.** Writing chunks to a tmpfile *inside* the final per-test dir lets `shutil.move` reduce to a POSIX `rename(2)` (atomic). If the SHA256 already exists on disk (re-upload of the same bytes), we drop the tmp and reuse — a fresh DB row records *who* uploaded it, even though no new bytes hit the disk. Saves storage AND preserves provenance.
- **Pyright's "underscore prefix unused" rule does not silence destructured tuple slots.** `ev, _test, scenario = chain` still triggers `"_test is not accessed"`. Workaround: use a single underscore (`_`) or index the tuple. Single underscore is conventional in Python for "I'm intentionally ignoring this".
- **TanStack v5 `useQueryClient.setQueryData(detailKey, next)`** is the right idiom after a mutation that returns the freshly-saved row — avoids a refetch, and the polling query still invalidates correctly on activity events. Pattern: `onSuccess: (next) => { qc.setQueryData(detailKey, next); qc.invalidateQueries({ queryKey: parentKey }); }`.
- **Activity polling must gate on `document.visibilityState === 'visible'`** or every backgrounded tab hits the API every 15 s, multiplying for free across a team's tab graveyard. Single-line check; massive impact.
- **PUT vs transition split kept the model coherent.** Tempting to fold "mark executed" into PUT `{state:'executed'}` but it conflates two concerns: state lifecycle vs field write. Keeping the transition POST separate makes the side-effect (`executed_at = now()`) easy to reason about and the perm gate per-target trivial.
- **`/diag/reset` must clean the evidence dir in test mode** otherwise the e2e suite accumulates 24 MB blobs across runs. Gated on `APP_ENV == "test"` so `dev` keeps the operator's manual uploads.
- **The `last_actor_id` migration adds an index on `updated_at`** — without it, the activity poll's `WHERE updated_at > since ORDER BY updated_at DESC` was sequential-scanning. With the index, the plan switches to an index range scan even on the empty case (which is the most common one when nothing has changed).
## 2026-05-13 — M6 missions + snapshot
- **Snapshot independence requires more than column copies — denormalise the join tables too.** `mission_tests` copies every scalar template field, but if `mission_test_mitre_tags` kept FKs to `mitre_*` rows, a future re-sync that drops a technique would cascade through `ON DELETE CASCADE` and silently mutate frozen missions. The M1 schema already split `mission_test_mitre_tags` with frozen `(mitre_external_id, mitre_name, mitre_url)` columns and no FK — at snapshot time we denormalise via a 3-query batch lookup (`_resolve_mitre_lookup`) and build the rows in-memory. Pattern to reuse for any "frozen reference" relationship in the future.
- **`/diag/reset` truncate order is FK-aware**: `mission_scenarios.source_scenario_template_id` and `mission_tests.source_test_template_id` are `ON DELETE SET NULL`. Truncating template tables first would force PG to NULL those columns one by one. Reverse the order — wipe mission tables (which cascade to members/scenarios/tests/tags/categories from `missions`) BEFORE the templates. Saves a round-trip + keeps the truncate logically aligned with the dependency graph.
- **Membership visibility = 404, not 403.** Returning 403 for "mission exists but you're not a member" leaks the existence of the mission. The service returns 404 in both "doesn't exist" and "not visible to you" cases via the same `MissionNotFound` exception. The decorator stack handles perm-level 403 (you can't even GET /missions); the service handles row-level 404. Pattern: gate "type of action" via decorator perms, gate "which rows" via service-level membership filters that collapse to 404.
- **Auto-add the non-admin creator as a member.** Without this, a redteamer who creates a mission and forgets to add themselves to `members[]` immediately loses visibility (403 on subsequent GETs because they're not a member). Solved at the service layer: `if not creator_is_admin and creator_id not in members: prepend (creator_id, 'red')`. Admin creators don't auto-add because they bypass membership anyway. Documented in the docstring + tested explicitly (`test_non_admin_creator_auto_added`).
- **Minimum-surface roster endpoint pattern**: `/users` returns admin metadata (is_admin via groups, is_active, group memberships). The mission wizard needs a list of assignable users from a non-admin redteamer's perspective — exposing /users to them would leak admin metadata. Added a dedicated `GET /users/roster` returning only `(id, email, display_name)` and gated by **any of** `user.read`, `mission.create`, `mission.update`. Pattern: when a cross-feature needs a smaller slice of an admin endpoint, create a dedicated lightweight endpoint rather than relaxing the admin one.
- **Pyright is not always wrong about "unused" parameters** — the original `_to_list_item(s: Session, m: Mission)` took `s` but never accessed it (the function uses already-`selectinload`ed relationships). Removed the param. Lesson: when adding a `Session` parameter to a view-assembly helper, audit whether the body actually issues queries through it.
- **`flask.abort()` is not typed `NoReturn` in this project's Pyright config** so `def f() -> X: if x is None: abort(...); return x` raises a return-type error. Workaround: add `assert user is not None` after the abort to narrow the type. Cleaner than `cast(...)`. Pattern to reuse anywhere we abort-and-return.
- **Snapshot of multiple scenarios is a 4-query write** regardless of test count: (1) load N scenario_templates with their join rows, (2) load M test_templates by id with mitre_tags, (3) batch-resolve MITRE rows (3 queries for tactic/technique/sub), (4) insert mission_scenarios + mission_tests + mission_test_mitre_tags via the SQLAlchemy unit of work. Avoid the temptation to query inside per-test loops — it explodes to O(scenarios × tests × tag_kinds) easily.
## 2026-05-12 — M5 templates + scenarios
- **`extra={"name": ...}` dans `log.info()` crash silencieusement** — Python's `logging.LogRecord` réserve `name` (le logger name). Coût : 500 sur le POST, message peu parlant (`KeyError: "Attempt to overwrite 'name' in LogRecord"`). Fix : renommer la clé (`template_name`). Liste réservée à éviter : `name`, `msg`, `args`, `levelname`, `levelno`, `pathname`, `filename`, `module`, `funcName`, `created`, `msecs`, `lineno`, `thread`, `threadName`, `process`. Pattern : préfixer les clés extra par l'entité (`template_name`, `group_id`, `user_id` est OK mais `id` aussi est piégeux dans certains setups).
- **React 18 + `setX((prev) => ({...prev, val: e.currentTarget.value }))` → page blanche au 1er input.** `e.currentTarget` est cleared après la fin du bubble, AVANT que l'updater fonctionnel exécute. Le synthetic event survit (pas de pooling depuis React 17), mais `currentTarget` est setté/cleared par le dispatcher. Fix : `e.target.value` (qui persiste sur le synthetic event), ou capturer `const v = e.currentTarget.value;` avant le `setX`. À garder en tête : tout `onChange` qui passe par un updater fonctionnel doit lire `e.target`, pas `e.currentTarget`.
- **Sentinel `Any = object()` plutôt que `... (Ellipsis)`** pour les "field unset" optional en service Python. Pyright voit `... = object()` correctement comme `Any`, alors que `description: str | None | object = ...` rend `description.strip()` invalide. Pattern : `_UNSET: Any = object()` au top du module + `description: Any = _UNSET` dans la signature + `if description is not _UNSET: ...`. Net + typecheck-friendly.
- **Postgres UNIQUE(scenario_id, position) + position-swap = ON CONFLICT pendant l'UPDATE.** Pour réordonner, le pattern naïf (UPDATE position) viole la contrainte sur le 1er swap. Trois options : (a) full delete + re-insert dans la même tx [retenu, atomique + lisible], (b) shift d'offset (UPDATE position = position + 1000 puis renumérotation), (c) deferred constraint. (a) gagne en simplicité — la liste rarement >50 éléments, le coût est négligeable.
- **`@dnd-kit/sortable` requires `useSortable({ id })` IDs to be unique and stable across renders.** Si on utilise un index numérique comme id, drag-and-drop ne réagit pas. Utiliser `test_template_id` (UUID stable) marche directement.
- **Frontend deps ajoutés à `package.json` sans `package-lock.json`** : le Dockerfile fait `npm install --no-audit --no-fund` sur fallback. OK pour M5 (3 deps `@dnd-kit/*`). À l'avenir, freeze un lockfile avant M14 pour build reproductibles.
- **Playwright `getByTestId` est défini par `testIdAttributeName: 'data-testid'`** dans `playwright.config.ts`. Pour qu'un test-id descende sur l'input via TextField, il faut que `...rest` soit spread sur l'input (déjà OK dans `TextField.tsx`). Mais avec un wrapper `<div><label/><input/></div>`, `getByTestId` matche le DIV si le test-id est dessus. Bien le mettre sur l'élément interactif (input/button), pas sur le container.
- **`/diag/reset` truncate order matters** : `scenario_template_tests.test_template_id` est FK `ON DELETE RESTRICT`, donc il faut truncate `scenario_template_tests` AVANT `test_templates`. Hierarchy : `scenario_template_tests → scenario_templates → test_template_mitre_tags → test_templates → mitre_*`. Maintenant inscrite dans `diag.py`.
- **Modal embarquant le `MitreTagPicker` complet (15 cols × 50 techniques)** : le picker se charge via `/mitre/matrix` (~94 KB). Affichage instantané, OK. Pour de futurs modals lourds, considérer le lazy-render derrière un toggle ou tab.
<!--
Template for future entries:
## YYYY-MM-DD — short title
- bullet
- bullet
-->