From a38fc0bfa9c6dda4ba137bba686bd9cbf24a0cd0 Mon Sep 17 00:00:00 2001 From: NADAL Jean-Baptiste Date: Mon, 14 Sep 2026 17:22:18 +0200 Subject: [PATCH] [FIX] Uge session of debugging and code coverage. --- .gitignore | 3 +- AGENTS.md | 127 +++- NAMING.md | 125 ++++ README.md | 24 +- TODO.md | 103 +++ api/.env | 2 - api/README.md | 18 + api/coverage-prepend.php | 25 + api/coverage-report.php | 64 ++ api/index.php | 118 ++-- api/lib/Auth.php | 97 +-- api/lib/ScoreScanner.php | 210 ++++-- api/router.php | 48 +- api/tests.php | 558 +++++++++++++-- deploy/deploy.sh | 4 +- partitions/.env | 1 - partitions/nginx.conf | 10 +- partitions/package-lock.json | 636 +++++++++++------- partitions/package.json | 9 +- partitions/src/lib/api.ts | 34 +- .../src/lib/components/ConfirmDialog.svelte | 75 +++ .../src/lib/components/PdfViewer.svelte | 49 +- partitions/src/lib/instruments.ts | 56 ++ partitions/src/lib/stores/auth.ts | 7 +- partitions/src/routes/+layout.svelte | 43 +- partitions/src/routes/admin/+page.svelte | 199 +----- partitions/src/routes/admin/[id]/+page.svelte | 152 ++--- .../src/routes/scores/[id]/+page.svelte | 71 +- .../routes/scores/[id]/[piece]/+page.svelte | 41 +- .../[id]/[piece]/[instrument]/+page.svelte | 73 +- partitions/src/routes/test-icons/+page.svelte | 83 --- partitions/static/favicon.png | Bin 0 -> 13758 bytes partitions/tests/e2e-smoke.mjs | 200 ++++++ partitions/vite.config.ts | 3 - test.sh | 139 ++++ 35 files changed, 2315 insertions(+), 1092 deletions(-) create mode 100644 NAMING.md create mode 100644 TODO.md delete mode 100644 api/.env create mode 100644 api/coverage-prepend.php create mode 100644 api/coverage-report.php delete mode 100644 partitions/.env create mode 100644 partitions/src/lib/components/ConfirmDialog.svelte create mode 100644 partitions/src/lib/instruments.ts delete mode 100644 partitions/src/routes/test-icons/+page.svelte create mode 100644 partitions/static/favicon.png create mode 100644 partitions/tests/e2e-smoke.mjs create mode 100755 test.sh diff --git a/.gitignore b/.gitignore index f3224d7..bd10a86 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ missing.xml partitions/.svelte-kit/ partitions/build/ partitions/node_modules/ -partitions/static/pdf.worker.min.js api/vendor/ api/composer.phar +api/.env +partitions/.env diff --git a/AGENTS.md b/AGENTS.md index 89defae..5573e03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,52 @@ site is archived in `legacy/` and must not be modified. - Node.js 26 / npm 11 - Svelte 5, SvelteKit 2, `@sveltejs/adapter-static`, Vite 5, Tailwind 3 - PHP 8.5 (CLI built-in server in development) -- Dependencies: `axios`, `pdfjs-dist`, `lucide-svelte` (front); `setasign/fpdi`, `setasign/fpdf`, `tecnickcom/tcpdf` (back) + +## Dependencies + +### Frontend (`partitions/package.json`) + +Runtime: + +| Package | Version | +|---|---| +| `axios` | ^1.6.0 | +| `pdfjs-dist` | ^4.0.379 | + +Dev: + +| Package | Version | +|---|---| +| `svelte` | ^5.0.0 | +| `@sveltejs/kit` | ^2.0.0 | +| `@sveltejs/adapter-static` | ^3.0.0 | +| `@sveltejs/vite-plugin-svelte` | ^4.0.0 | +| `vite` | ^5.0.0 | +| `typescript` | ^5.0.0 | +| `svelte-check` | ^4.0.0 | +| `tailwindcss` | ^3.4.0 | +| `autoprefixer` | ^10.4.16 | +| `postcss` | ^8.4.32 | + +Install: `cd partitions && npm install`. + +### Backend (`api/composer.json`) + +| Package | Version | +|---|---| +| `setasign/fpdi` | ^2.6 (import de pages PDF) | +| `setasign/fpdf` | ^1.8 (base PDF) | +| `tecnickcom/tcpdf` | ^6.10 | + +Install: `cd api && php composer.phar install` (le dossier `vendor/` est déjà présent). + +PHP extensions requises (via `composer.lock`) : `gd`, `zlib`, `curl`. Déjà fournies +ou activées avec les paquets Arch `php` + `php-gd`. + +### Tooling de développement + +- `xdebug` (paquet Arch `extra/xdebug`) — optionnel, utilisé pour mesurer la + couverture de code du backend (`xdebug.mode=coverage`). > PHP 8.5 note: nullable parameters MUST use explicit `?type` syntax > (e.g. `?string $x = null`). Implicit nullable is deprecated and pollutes API output. @@ -46,12 +91,14 @@ cd partitions npm install npm run dev # http://localhost:5173 npm run build # outputs to partitions/build +npm run check # svelte-check (0 errors / 0 warnings expected) npm run preview ``` - `frontend` is a symlink to `partitions/build` (the deployed static site). +- `VITE_API_URL` (build/dev env) selects the API base URL; falls back to + `localhost:8000` (dev) or the production URL. - No ESLint / test framework is configured for the frontend. -- `VITE_API_URL` is defined but currently unused (base URL is chosen from `import.meta.env.DEV`). ### Deploy @@ -65,6 +112,17 @@ npm run preview ## Testing +### Run everything (single command) + +```bash +./test.sh +``` + +Runs, in order: backend tests (`api/tests.php`), backend code coverage (Xdebug, +if available), frontend `npm run check` (svelte-check), and the frontend E2E smoke +test. It starts the API and dev servers if needed and stops them afterwards. +"Test the project" means running `./test.sh`. + ### PHP API tests ```bash @@ -80,6 +138,49 @@ php tests.php validation, INI injection, security headers. - `tests.php` is kept for development only and MUST NOT be shipped in the deploy zip. +### Code coverage (optional, requires Xdebug) + +The test suite covers ~96% of `api/` non-vendor code (`Auth.php` and `router.php` +at 100%, `ScoreScanner.php` +99%). The remaining uncovered lines are defensive +I/O-failure branches (rate-limiter `flock`/`fopen` failures, missing +`SCORES_PATH`, `OOM`/partial PHP upload errors, `createScore` rollback) that +need OS-level fault injection to trigger. + +```bash +cd api +rm -rf /tmp/ohmj_cov +# Terminal 1: instrumented dev server +php -d zend_extension=xdebug -d xdebug.mode=coverage \ + -d auto_prepend_file="$PWD/coverage-prepend.php" \ + -c php-upload.ini -S localhost:8000 router.php +# Terminal 2: run tests through the instrumented server +php -d zend_extension=xdebug -d xdebug.mode=coverage \ + -d auto_prepend_file="$PWD/coverage-prepend.php" tests.php +# Then, from the repo root: +SHOW_MISSING=1 php api/coverage-report.php \ + api/index.php api/router.php api/lib/Auth.php api/lib/ScoreScanner.php +``` + +`coverage-prepend.php` / `coverage-report.php` are dev-only and excluded from the +deploy archive. + +### Frontend E2E smoke test (CDP, no dependency) + +Requires the API (`:8000`) and the frontend dev server (`:5173`) to be running. + +```bash +cd partitions +npm run test:e2e +``` + +`tests/e2e-smoke.mjs` drives the system `chromium` through the DevTools Protocol: +it logs in via the API, injects the session, then visits `/scores`, a score +detail, the PDF viewer and `/admin`. It asserts that every API call returns +`< 400`, that the SPA stays on each route, and that the PDF is fetched with an +`Authorization` header (no token in the URL). + +Env overrides: `API_URL`, `FRONT_URL`, `CHROME_PATH`, `LOGIN_USER`, `LOGIN_PASS`. + ## Authentication - Login: `POST /login` with `{"username":"admin","password":"password"}` (development). @@ -137,6 +238,9 @@ Admin only (`role === 'admin'`): ## Data Model (files) +> Full filename/directory convention: see [`NAMING.md`](NAMING.md). It is +> normalized and MUST NOT be changed casually (backend + frontend depend on it). + ``` legacy/Scores/////.pdf legacy/Scores//score.ini @@ -218,11 +322,20 @@ curl -s -I http://localhost:8000/login -X POST \ -d '{"username":"admin","password":"password"}' ``` -> Known security findings from the latest review (not yet fixed): static file -> passthrough in `api/router.php` exposes `.env`/`users.json`; `deleteScoreFile` -> lacks path containment; `api/.env` is tracked by git; JWT is standard base64 -> (not base64url) and expiry is optional; stored XSS via the `ressource` link. -> See the project issue tracker / recent review notes. +> Security fixes applied recently: `api/router.php` no longer serves files +> statically (`.env`/`config/users.json` are not reachable); `deleteScoreFile` +> enforces `realpath()` containment and only deletes `.pdf`; `api/.env` and +> `partitions/.env` are untracked/gitignored (rotate the secret since it remains +> in git history); JWT uses base64url, requires `exp` and validates `alg`; +> credentials errors are generic; the `ressource` link rejects non-http(s) URLs. +> +> Known findings still open: default `admin`/`password` credentials; login rate +> limiter is race-prone (no `flock`) and `/api/login` bypasses it; CORS falls +> back to `*` for unknown origins; `GET /admin/scores/:id` passes extra +> arguments dropped by `ScoreScanner::updateScore` (so `ressource` is not +> persisted); watermarking only keeps page 1 of multi-page PDFs; `createScore` +> and `uploadPdf` do not validate path segments; `updateScore`/`updatePieces` +> do not sanitize INI values. ## Temporary Files diff --git a/NAMING.md b/NAMING.md new file mode 100644 index 0000000..530a97d --- /dev/null +++ b/NAMING.md @@ -0,0 +1,125 @@ +# NAMING.md - Convention des noms de partitions + +Référence du format de fichiers/répertoires des partitions (`legacy/Scores/`). +Cette convention a été normalisée une fois par `scripts/convert_final_v2.js` +(6388 fichiers convertis). **Ne pas renommer à la main** : le backend +(`api/lib/ScoreScanner.php`) et le frontend en dépendent. + +## Arborescence + +``` +legacy/Scores///// +legacy/Scores//score.ini +``` + +| Segment | Rôle | Exemples | +|---|---|---| +| `NUM` | identifiant du morceau, **zéro-paddé sur 3 chiffres** | `102`, `390`, `072`, `098` | +| `PIECE` | numéro de pièce (mouvement) | `1` (morceau simple), `1`..`5` (medley type `390`) | +| `INSTRUMENT` | trigramme de l'instrument | `cla`, `sax`, `sup`… (voir table) | +| `VERSION` | répertoire de version | `1` = version principale, `2`, `3`… = alternatives | +| `FICHIER` | nom du PDF (grammaire ci-dessous) | `clarinette-sib-1.pdf` | + +> `VERSION` est un **répertoire**, pas un token du nom de fichier. Le dossier `1/` +> est la version principale ; `2/`+ sont des versions alternatives du même morceau +> (ex. score `210` : `cla/1/` et `cla/2/`). + +## Grammaire du nom de fichier + +``` +{instrument}[-{tonalité}][-{variante}]-{partie}[_{partie_combinée}][_{clef}][_alt{N}].pdf +``` + +- Séparateur de champs : **`-`** +- Séparateur interne aux noms composés et aux suffixes : **`_`** +- Multi-instrument (deux instruments sur la même partition) : **`+`** + +### Exemples réels + +| Fichier | Sens | +|---|---| +| `clarinette-sib-1.pdf` | clarinette en Si♭, partie 1 | +| `clarinette_alto-mib-1.pdf` | clarinette alto en Mi♭, partie 1 (`_` = nom composé) | +| `sax_alto-mib-2.pdf` | sax alto en Mi♭, partie 2 | +| `cor-fa-1_2.pdf` | cor en Fa, parties **1 et 2 combinées** | +| `baryton-clefa-1.pdf` | baryton, clé de fa, partie 1 (pas de tonalité) | +| `euphonium-sib-clesol-1.pdf` | euphonium Si♭, clé de sol, partie 1 | +| `cornet-sib-solo-1.pdf` | variante `solo` | +| `baryton-sib-default_cor-2.pdf` | substitution `default_cor` | +| `conducteur-1_alt1.pdf` | version alternative `alt1` | +| `basse-sib-1+contrebasse-sib-1.pdf` | **deux instruments** sur la même partition | +| `saxophone-mib-1+baryton-mib-1.pdf` | idem (sax + baryton) | +| `glockenspiel-1+xylophone-1.pdf` | percussion multiple | +| `percussion-1+gong-1+cymbale-1+triangle-1.pdf` | 3 `+` possibles | + +### Tonalités + +`sib` (Si♭), `mib` (Mi♭), `fa` (Fa), `ut` (Do — **`do` s'écrit `ut` dans les +fichiers**), `sol`, `re`, `mi`, `si`, `reb`, `dob`, `solb`, `lab`. + +### Clefs + +`clesol` (clé de sol), `clefa` (clé de fa). + +### Variantes / suffixes + +- `solo` : partie solo. +- `default_` : instrument de substitution (ex. `default_cor`). +- `_altN` : version alternative dans un même dossier (ex. `_alt1`, `_alt2`). + +## Codes instruments + +| Code | Instrument | Code | Instrument | +|---|---|---|---| +| `dir` | Direction | `trp` | Trompette | +| `pic` | Piccolo | `trb` | Trombone | +| `flu` | Flûte | `tub` | Tuba | +| `cla` | Clarinette | `htb` | Hautbois | +| `clb` | Clarinette basse | `bas` | Basson | +| `sax` | Saxophone alto | `per` | Percussions | +| `sat` | Saxophone ténor | `crn` | Cornet | +| `sab` | Saxophone baryton | `eup` | Euphonium | +| `coa` | Cor anglais | `har` | Harpe | +| `cba` | Contrebasse | `pia` | Piano | +| `cor` | Cor | `sup` / `par` | Parties combinées | + +## `score.ini` + +```ini +[info] +name = La part d'Euterpe +compositor = Michaël CUVILLON +ressource = https://... ; optionnel (lien externe) + +[pieces] +count = 1 +; pour un medley (count = N) : +; 1 = La Part d Euterpe +; 2 = Calliope s Songs +``` + +- `[pieces] count` = nombre de pièces. Les clés `1`..`N` donnent leur nom. +- `ressource` : lien externe optionnel (YouTube, éditeur…). + +## Points d'attention (code) + +- **`+` dans les chemins** : un `+` littéral dans une URL est valide en *path*, + mais PHP `urldecode()` le transforme en espace. Le download doit donc : + - encoder chaque segment côté frontend (`encodePath` dans `partitions/src/lib/api.ts`) ; + - utiliser `rawurldecode()` (jamais `urldecode()`) côté `api/index.php`. + Sinon les 167 fichiers multi-instruments (`a+b.pdf`) renvoient 403. +- **Parsing** : `ScoreScanner::parseFilename()` détecte tonalité, clef, variante + et partie. Les motifs doivent être ancrés (`_sib(?:_|$)`) et du plus long au plus + court (`sibemol` avant `sib`) pour ne pas mal découper les noms. +- Le matcher multi-instruments utilise `+` (et non `_et_`, supprimé par la + conversion). + +## Conversion (one-off) + +`scripts/convert_final_v2.js` est le convertisseur historique utilisé pour +normaliser l'ensemble du catalogue. Il ne doit plus être nécessaire pour de +nouveaux fichiers (uploader via l'admin respecte la convention). + +```bash +node scripts/convert_final_v2.js /chemin/vers/Scores confirm +``` diff --git a/README.md b/README.md index 10412b9..6a8b98b 100644 --- a/README.md +++ b/README.md @@ -40,10 +40,26 @@ Le site déployé est exposé via le lien symbolique `frontend -> partitions/bui ## Tests +Tout en une commande (backend + couverture + frontend) : + +```bash +./test.sh +``` + +Détail manuel : + ```bash cd api rm -f /tmp/rate_* 2>/dev/null -php tests.php # attendu : 53/53 +php tests.php # attendu : 181/181 +``` + +Frontend (serveurs API + dev lancés) : + +```bash +cd partitions +npm run check # svelte-check : 0 erreur / 0 warning +npm run test:e2e # smoke E2E navigateur (CDP) ``` ## Déploiement @@ -55,6 +71,12 @@ php tests.php # attendu : 53/53 Le build frontend doit être présent dans `frontend/`. `legacy/` et `api/tests.php` ne sont pas inclus dans l'archive. +## Documentation + +- [`AGENTS.md`](AGENTS.md) — guide de développement (stack, commandes, conventions) +- [`NAMING.md`](NAMING.md) — convention des noms de partitions (`legacy/Scores`) +- [`api/README.md`](api/README.md) — documentation de l'API REST + ## Licence GPL v2 diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..61cf3d2 --- /dev/null +++ b/TODO.md @@ -0,0 +1,103 @@ +# TODO.md - Plan de travail en cours + +> Document de reprise : si la session est interrompue, reprendre ici. +> Dernière mise à jour : 2026-09-14. + +## Comment lancer / tester + +```bash +# Tout tester d'un coup (backend + couverture + frontend) : "test le projet" +./test.sh + +# Serveur API (dev) - obligatoire pour les tests +cd api && php -c php-upload.ini -S localhost:8000 router.php & +# Front (dev) +cd partitions && npm run dev # http://localhost:5173 + +# Tests backend +cd api && rm -f /tmp/rate_* 2>/dev/null && php tests.php + +# Tests frontend +cd partitions && npm run check && npm run test:e2e + +# Couverture (Xdebug requis) ~96.9% +``` + +## État actuel + +### Fait (session) +- **Docs** : `AGENTS.md` réécrit (stack réelle), `README.md` + `api/README.md` corrigés, + `NAMING.md` recréé (convention des noms), dépendances documentées, `PLAN.md` / + `SECURITY_AUDIT.md` / `wordpress_list/` supprimés (commit `e79c865`). +- **Sécurité C1-C5** : plus de service statique `.env` (`router.php`), containment + `deleteScoreFile`, `.env` dé-suivis + secret roté, JWT base64url + `exp` obligatoire + + `alg` validé + erreurs génériques, XSS `ressource` bloqué. +- **Bugs** : `+`/encodage des chemins (167 PDF), viewer PDF (course canvas + double + chargement + fuite blob), en-tête `Authorization` insensible à la casse, 404/403 download. +- **Backend** : `ressource` persisté/préservé, watermark multi-pages + échec remonté, + validation des segments de chemin, sanitisation INI, rate-limiter `flock`, + CORS sans wildcard, token download via header uniquement, `ini_set` retiré, + `score.ini` protégé, code mort supprimé (Auth methods). +- **Tests** : `tests.php` porte de 53 à **181 tests** (fonctionnels + unitaires directs). + Tests isolés (créent/suppriment leurs propres partitions) ; le test watermark utilisait + un PDF invalide et écrasait `scores[0]` → corrigé. +- **Harness couverture** : `api/coverage-prepend.php` + `api/coverage-report.php` + (exclus du deploy), documentés dans `AGENTS.md`. + +### Couverture backend : 96.9% +- `api/router.php` : **100%** +- `api/lib/Auth.php` : **100%** +- `api/lib/ScoreScanner.php` : **99.5%** (restent 2 lignes) +- `api/index.php` : **92.4%** (restent 22 lignes) + +Code inutile retiré (justifié en revue) : fallback `getallheaders()` (lit la même +source que `$_SERVER['HTTP_AUTHORIZATION']`) et boucle d'auto-id morte (comparait +un id non paddé à des dossiers paddés). + +Lignes non couvertes = **branches défensives d'échec I/O**, non atteignables via +l'API sans injection de panne OS : + +| Fichier | Lignes | Nature | +|---|---|---| +| `api/index.php` | 56, 59, 60 | rate-limiter : `fopen`/`flock` échoue (fail-open) | +| `api/index.php` | 104 | fallback `SCORES_PATH` si variable absente | +| `api/index.php` | 222-224 | `GET /scores` renvoie 500 si dossier Scores absent | +| `api/index.php` | 388-408 | erreurs d'upload PHP (`UPLOAD_ERR_*`, post_max_size) | +| `api/lib/ScoreScanner.php` | 443, 444 | rollback `createScore` si écriture `score.ini` échoue | + +**Option** pour monter plus haut : serveur de test secondaire (port 8001) avec +`SCORES_PATH=/nonexistent` et `upload_max_filesize=1K` pour couvrir 222-224 et +388-395. Le reste (I/O/OS, 56/59/60, 104, 443/444) resterait non couvert. + +## À faire (reste) + +### Finitions +- [ ] Décider du sort de `legacy/Scores/001/` (résidu de test « Test Duplicate », + sans PDF) → demander avant suppression. +- [ ] `npm audit` : 18 → **8 vulnérabilités** après `npm audit fix`. Le reste + (svelte/rollup/picomatch) exige `npm audit fix --force` (breaking) → à décider. +- [ ] Committer (uniquement sur demande). + +## Fait (frontend, F11-F16) +- **F11** code mort supprimé : `auth.getToken()`, `api.downloadFileWithAuth`, + tout le code d'upload inutilisé d'`admin/+page.svelte`. +- **F12** maps d'instruments factorisées dans `partitions/src/lib/instruments.ts` + (+ helpers `getInstrumentName/Icon/DefaultKey`), réutilisées partout. `test-icons` + supprimé. `lucide-svelte` retiré (inutilisé). +- **F13** migration `$app/stores` → `$app/state`, suppression des abonnements + manuels, état réactif dans `scores/[id]` et chargement piloté par `$effect` + (navigation client correcte). `` → `{@render children()}`. +- **F14** worker PDF pointé sur `.mjs` (fichier suivi), `nginx.conf` MIME `.mjs`, + `deploy.sh` simplifié, `favicon.png` ajouté. +- **F15** route `test-icons/` supprimée ; `VITE_API_URL` réellement utilisé + (fallback dev/prod), `define` bogus retiré de `vite.config.ts`. +- **F16** `svelte-check` ajouté (`npm run check`) → **0 erreur / 0 warning** ; + a11y corrigée (labels `for`/`id`, ``/spans cliquables → ` + + +{/if} diff --git a/partitions/src/lib/components/PdfViewer.svelte b/partitions/src/lib/components/PdfViewer.svelte index 8be0b05..121b3c4 100644 --- a/partitions/src/lib/components/PdfViewer.svelte +++ b/partitions/src/lib/components/PdfViewer.svelte @@ -1,27 +1,20 @@ @@ -263,8 +135,9 @@
{ e.preventDefault(); createScore(); }} class="space-y-4">
- +
- +
- +
{#if newPieceCount > 1} -
- +
+ Nom des parties {#each newPieceNames as pieceName, i} {/each} -
+ {/if}