Files
Metamorph/tasks/lessons.md

124 lines
28 KiB
Markdown
Raw Normal View History

2026-05-11 06:16:00 +02:00
---
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 06:16:00 +02:00
## 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.
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
## 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).
feat(m6): missions + snapshot CRUD, membership visibility, status state machine Adds the mission layer that materialises template snapshots, plus the SPA list / 3-step wizard / detail page. Backend: - app/services/missions.py — create_mission snapshots scenarios, tests, MITRE tags in a 4-query write; list/get apply a non-admin membership filter that collapses to 404 (no existence leak); status state machine enforces draft → in_progress → completed → archived with archived as a sink; the non-admin creator is auto-added as role_hint='red' to retain visibility. - app/api/missions.py — 8 endpoints (list, get, create, update, add scenarios, set members, transition, soft-delete) with strict pydantic schemas. The transition endpoint splits the perm gate manually so archive requires mission.archive while other targets use mission.update. - app/api/users.py — new GET /users/roster returning (id, email, display_name) only, gated by user.read OR mission.create OR mission.update — lets non-admin wizard users see assignable peers without exposing the admin /users payload. - app/api/diag.py — /diag/reset truncates the mission_* tables before the template tables because the source_*_template_id FKs are ON DELETE SET NULL, which is cheaper to short-circuit by removing the children first. Frontend: - lib/missions.ts — typed client, queryKey factory, status accent map. - pages/MissionsListPage.tsx — list cards with status accent + filters (q, client, status). - pages/MissionsCreatePage.tsx — 3-step wizard (meta → scenarios → members) with member roster fed by /users/roster. - pages/MissionDetailPage.tsx — header + transition buttons (legal next states only) + Tests/Members/Synthesis/Export tabs. - Routes + nav entry (visible to anyone with mission.read or admin). Tests: - backend/tests/test_missions.py — 22 pytest covering snapshot fidelity, MITRE propagation, membership visibility, transition state machine, perm gating, member set replace, append scenarios, soft-delete, partial update, inverted-date rejection. - e2e/tests/m6-missions.spec.ts — 5 Playwright (snapshot freezing, non-admin visibility, status transitions + 409, SPA wizard end-to-end, list filter). Docs: - CHANGELOG, tasks/testing-m6.md, tasks/lessons.md (snapshot tradeoffs, membership=404 pattern, /diag/reset order, auto-creator add). - README + tasks/todo.md updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 15:07:32 +02:00
## 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.
2026-05-11 06:16:00 +02:00
<!--
Template for future entries:
## YYYY-MM-DD — short title
- bullet
- bullet
-->