refactor(api): brancher validate.js, qui était du code mort bien testé

Huit des douze exports de validate.js n'étaient appelés nulle part en
production : app.js et adminRoutes.js réimplémentaient la même validation à la
main, trois fois. Les 64 cas de validate.test.js et validate.property.test.js
garantissaient donc une implémentation qui ne tournait nulle part — le score de
mutation de 90 % portait sur du code inatteignable.

Une divergence s'était déjà installée sans que rien ne rougisse : parseMaId
exige un entier là où le contrôle en ligne se contentait de Number.isFinite.
/api/band/1.5 était accepté et partait en base pour ne rien trouver ; il répond
désormais 400.

Les routes délèguent maintenant à parseBbox, parseZoom, cellSizeForZoom,
parseCsvList, parseYear, parseLat, parseLon et parseMaId. Aucun test existant
n'a bougé, ce qui confirme que le comportement est identique partout ailleurs.

Ajoute validateWiring.test.js, qui échoue si un helper cesse d'être branché. Il
ne dit rien de sa qualité — c'est le rôle des deux autres fichiers — seulement
qu'il est réellement sur le chemin d'exécution. C'est exactement le garde-fou
qui manquait pour que la dérive ne recommence pas.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Nicolas FRYDER 2026-08-22 14:45:54 +02:00
parent e35e3d6de3
commit a5de45dd9f
3 changed files with 159 additions and 111 deletions

View file

@ -1,5 +1,5 @@
import { requireAdminSession, writeAuditLog } from "./adminAuth.js"; import { requireAdminSession, writeAuditLog } from "./adminAuth.js";
import { pagination } from "./validate.js"; import { ValidationError, pagination, parseLat, parseLon, parseYear } from "./validate.js";
const BAND_SORT_COLUMNS = new Set([ const BAND_SORT_COLUMNS = new Set([
"ma_id", "name", "country", "status", "genre", "ma_id", "name", "country", "status", "genre",
@ -248,26 +248,18 @@ export default async function adminRoutes(fastify, opts) {
if (Object.keys(updates).length === 0) { if (Object.keys(updates).length === 0) {
return reply.code(400).send({ ok: false, error: "Aucun champ à modifier" }); return reply.code(400).send({ ok: false, error: "Aucun champ à modifier" });
} }
if ("formed_year" in updates) { // Mêmes règles que les routes publiques, et une seule implémentation :
const y = updates.formed_year === null ? null : Number(updates.formed_year); // elles étaient recopiées à la main ici alors que validate.js les portait
if (y !== null && (!Number.isFinite(y) || y < 1800 || y > 2100)) { // déjà, testées par 64 cas qui ne couvraient donc pas ce qui tournait.
return reply.code(400).send({ ok: false, error: "formed_year invalide" }); try {
if ("formed_year" in updates) updates.formed_year = parseYear(updates.formed_year, "formed_year");
if ("lat" in updates) updates.lat = parseLat(updates.lat);
if ("lon" in updates) updates.lon = parseLon(updates.lon);
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
} }
updates.formed_year = y; throw err;
}
if ("lat" in updates) {
const v = updates.lat === null ? null : Number(updates.lat);
if (v !== null && (!Number.isFinite(v) || v < -90 || v > 90)) {
return reply.code(400).send({ ok: false, error: "lat invalide" });
}
updates.lat = v;
}
if ("lon" in updates) {
const v = updates.lon === null ? null : Number(updates.lon);
if (v !== null && (!Number.isFinite(v) || v < -180 || v > 180)) {
return reply.code(400).send({ ok: false, error: "lon invalide" });
}
updates.lon = v;
} }
// Transaction : l'edition, la pose de l'override de localisation et // Transaction : l'edition, la pose de l'override de localisation et
@ -766,17 +758,20 @@ export default async function adminRoutes(fastify, opts) {
return reply.code(400).send({ ok: false, error: "bad id" }); return reply.code(400).send({ ok: false, error: "bad id" });
} }
const { lat, lon } = req.body || {}; const { lat, lon } = req.body || {};
const latN = lat === null || lat === undefined || lat === "" ? null : Number(lat); let latN, lonN;
const lonN = lon === null || lon === undefined || lon === "" ? null : Number(lon); try {
latN = parseLat(lat);
lonN = parseLon(lon);
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
}
throw err;
}
// parseLat/parseLon acceptent null (effacement) ; ici les deux sont requis.
if (latN === null || lonN === null) { if (latN === null || lonN === null) {
return reply.code(400).send({ ok: false, error: "lat et lon requis" }); return reply.code(400).send({ ok: false, error: "lat et lon requis" });
} }
if (!Number.isFinite(latN) || latN < -90 || latN > 90) {
return reply.code(400).send({ ok: false, error: "lat invalide" });
}
if (!Number.isFinite(lonN) || lonN < -180 || lonN > 180) {
return reply.code(400).send({ ok: false, error: "lon invalide" });
}
const before = await pool.query(`SELECT * FROM band_locations WHERE id = $1`, [id]); const before = await pool.query(`SELECT * FROM band_locations WHERE id = $1`, [id]);
if (!before.rows.length) return reply.code(404).send({ ok: false, error: "not found" }); if (!before.rows.length) return reply.code(404).send({ ok: false, error: "not found" });

View file

@ -22,7 +22,17 @@ import {
requireAdminSession, requireAdminSession,
} from "./adminAuth.js"; } from "./adminAuth.js";
import adminApiRoutes from "./adminRoutes.js"; import adminApiRoutes from "./adminRoutes.js";
import { ValidationError, sanitizeSearchString, parseLimitOffset } from "./validate.js"; import {
ValidationError,
sanitizeSearchString,
parseLimitOffset,
parseBbox,
parseZoom,
cellSizeForZoom,
parseCsvList,
parseYear,
parseMaId,
} from "./validate.js";
/** /**
* @param {object} [opts] * @param {object} [opts]
@ -282,36 +292,23 @@ export async function buildServer(opts = {}) {
year_max, year_max,
} = /** @type {Record<string, string|undefined>} */ (req.query || {}); } = /** @type {Record<string, string|undefined>} */ (req.query || {});
if (!bbox || !zoom) { // Validation déléguée à validate.js. Ces règles y étaient déjà écrites ET
return reply.code(400).send({ ok: false, error: "bbox and zoom are required" }); // couvertes par 64 tests, mais la production les réimplémentait à la main
// ici : les tests garantissaient donc une implémentation qui ne tournait
// nulle part. Une divergence existait déjà (parseMaId exige un entier, le
// code en ligne acceptait un flottant).
let minLon, minLat, maxLon, maxLat, zoomLevel, cellSize;
try {
({ minLon, minLat, maxLon, maxLat } = parseBbox(bbox));
zoomLevel = parseZoom(zoom);
cellSize = cellSizeForZoom(zoomLevel);
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
} }
throw err;
const bboxParts = String(bbox).split(",").map(Number);
if (bboxParts.length !== 4) {
return reply.code(400).send({ ok: false, error: "bbox must have 4 values" });
} }
const [minLon, minLat, maxLon, maxLat] = bboxParts;
const zoomLevel = Number(zoom);
if ([minLon, minLat, maxLon, maxLat, zoomLevel].some(v => !Number.isFinite(v))) {
return reply.code(400).send({ ok: false, error: "Invalid bbox or zoom values" });
}
if (minLat < -90 || maxLat > 90 || minLon < -180 || maxLon > 180) {
return reply.code(400).send({ ok: false, error: "Coordinates out of range" });
}
if (minLon >= maxLon || minLat >= maxLat) {
return reply.code(400).send({ ok: false, error: "Invalid bbox bounds" });
}
if (zoomLevel < 0 || zoomLevel > 22) {
return reply.code(400).send({ ok: false, error: "Zoom level must be between 0 and 22" });
}
const cellSize = Math.max(0.01, 180 / Math.pow(2, zoomLevel));
// Source: band_locations (un point par localisation géocodée) JOIN bands. // Source: band_locations (un point par localisation géocodée) JOIN bands.
const where = [ const where = [
"bl.geom IS NOT NULL", "bl.geom IS NOT NULL",
@ -324,11 +321,9 @@ export async function buildServer(opts = {}) {
vals.push(minLon, minLat, maxLon, maxLat); vals.push(minLon, minLat, maxLon, maxLat);
i += 4; i += 4;
try {
if (countries) { if (countries) {
const cs = String(countries).split(",").map(s => s.trim().toUpperCase()).filter(Boolean); const cs = parseCsvList(countries, { max: 100, label: "pays", transform: (x) => x.toUpperCase() });
if (cs.length > 100) {
return reply.code(400).send({ ok: false, error: "Max 100 pays simultanés" });
}
if (cs.length) { if (cs.length) {
where.push(`b.country = ANY($${i}::text[])`); where.push(`b.country = ANY($${i}::text[])`);
vals.push(cs); vals.push(cs);
@ -337,16 +332,19 @@ export async function buildServer(opts = {}) {
} }
if (status) { if (status) {
const st = String(status).split(",").map(s => s.trim()).filter(Boolean); const st = parseCsvList(status, { max: 50, label: "statuts" });
if (st.length > 50) {
return reply.code(400).send({ ok: false, error: "Max 50 statuts simultanés" });
}
if (st.length) { if (st.length) {
where.push(`COALESCE(NULLIF(trim(b.status), ''), 'Unknown') = ANY($${i}::text[])`); where.push(`COALESCE(NULLIF(trim(b.status), ''), 'Unknown') = ANY($${i}::text[])`);
vals.push(st); vals.push(st);
i++; i++;
} }
} }
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
}
throw err;
}
if (genre) { if (genre) {
try { try {
@ -359,25 +357,25 @@ export async function buildServer(opts = {}) {
} }
} }
if (year_min) { try {
const yearMin = Number(year_min); const yearMin = parseYear(year_min, "year_min");
if (!Number.isFinite(yearMin) || yearMin < 1800 || yearMin > 2100) { if (yearMin !== null) {
return reply.code(400).send({ ok: false, error: 'year_min invalide' });
}
where.push(`b.formed_year >= $${i}`); where.push(`b.formed_year >= $${i}`);
vals.push(yearMin); vals.push(yearMin);
i++; i++;
} }
const yearMax = parseYear(year_max, "year_max");
if (year_max) { if (yearMax !== null) {
const yearMax = Number(year_max);
if (!Number.isFinite(yearMax) || yearMax < 1800 || yearMax > 2100) {
return reply.code(400).send({ ok: false, error: 'year_max invalide' });
}
where.push(`b.formed_year <= $${i}`); where.push(`b.formed_year <= $${i}`);
vals.push(yearMax); vals.push(yearMax);
i++; i++;
} }
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
}
throw err;
}
const whereSql = where.join(" AND "); const whereSql = where.join(" AND ");
@ -526,32 +524,40 @@ export async function buildServer(opts = {}) {
const vals = []; const vals = [];
let i = 1; let i = 1;
try {
if (countries) { if (countries) {
const cs = String(countries).split(",").map(s => s.trim().toUpperCase()).filter(Boolean); const cs = parseCsvList(countries, { max: 100, label: "pays", transform: (x) => x.toUpperCase() });
if (cs.length > 100) {
return reply.code(400).send({ ok: false, error: "Max 100 pays simultanés" });
}
if (cs.length) { if (cs.length) {
where.push(`country = ANY($${i}::text[])`); where.push(`country = ANY($${i}::text[])`);
vals.push(cs); vals.push(cs);
i++; i++;
} }
} }
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
}
throw err;
}
if (geocoded === "1") where.push(`geom IS NOT NULL`); if (geocoded === "1") where.push(`geom IS NOT NULL`);
if (geocoded === "0") where.push(`geom IS NULL`); if (geocoded === "0") where.push(`geom IS NULL`);
try {
if (status) { if (status) {
const st = String(status).split(",").map(s => s.trim()).filter(Boolean); const st = parseCsvList(status, { max: 50, label: "statuts" });
if (st.length > 50) {
return reply.code(400).send({ ok: false, error: "Max 50 statuts simultanés" });
}
if (st.length) { if (st.length) {
where.push(`COALESCE(NULLIF(trim(status), ''), 'Unknown') = ANY($${i}::text[])`); where.push(`COALESCE(NULLIF(trim(status), ''), 'Unknown') = ANY($${i}::text[])`);
vals.push(st); vals.push(st);
i++; i++;
} }
} }
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
}
throw err;
}
if (only_black === "1") { if (only_black === "1") {
where.push(`genre ILIKE '%black%'`); where.push(`genre ILIKE '%black%'`);
@ -667,10 +673,18 @@ export async function buildServer(opts = {}) {
try { try {
const p = requirePool(); const p = requirePool();
const { ma_id } = /** @type {{ ma_id: string }} */ (req.params); const { ma_id } = /** @type {{ ma_id: string }} */ (req.params);
const id = Number(ma_id); let id;
try {
if (!Number.isFinite(id) || id < 0) { // parseMaId exige un ENTIER. Le contrôle en ligne se contentait de
return reply.code(400).send({ ok: false, error: "bad ma_id" }); // Number.isFinite : /api/band/1.5 passait la validation et partait en
// base pour ne rien trouver. C'est la divergence que la duplication
// avait laissée s'installer.
id = parseMaId(ma_id);
} catch (err) {
if (err instanceof ValidationError) {
return reply.code(400).send({ ok: false, error: err.message });
}
throw err;
} }
const r = await p.query( const r = await p.query(

View file

@ -0,0 +1,39 @@
import { describe, it, expect } from "vitest";
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
/**
* validate.js ne doit pas redevenir du code mort bien testé.
*
* Huit de ses douze exports n'étaient appelés nulle part en production :
* app.js et adminRoutes.js réimplémentaient la même validation à la main, trois
* fois. Les 64 tests de validate.test.js et validate.property.test.js
* garantissaient donc une implémentation qui ne tournait nulle part et une
* divergence s'était déjà installée sans que rien ne rougisse (parseMaId exige
* un entier, le contrôle en ligne acceptait un flottant, si bien que
* /api/band/1.5 était accepté).
*
* Ce test échoue si un helper cesse d'être branché. Il ne dit rien de sa
* qualité c'est le rôle des deux autres fichiers — seulement qu'il est
* réellement sur le chemin d'exécution.
*/
const SRC = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../src");
const lire = (f) => fs.readFileSync(path.join(SRC, f), "utf8");
const validate = lire("validate.js");
const consommateurs = ["app.js", "adminRoutes.js"].map(lire).join("\n");
const exports = [...validate.matchAll(/export\s+(?:async\s+)?(?:class|function)\s+(\w+)/g)]
.map((m) => m[1]);
describe("branchement de validate.js", () => {
it("expose au moins les douze helpers connus", () => {
expect(exports.length).toBeGreaterThanOrEqual(12);
});
it.each(exports)("%s est réellement utilisé en production", (nom) => {
const utilise = new RegExp(`\\b${nom}\\b`).test(consommateurs);
expect(utilise).toBe(true);
});
});