Durcir la comparaison du HEALTH_TOKEN (timing-safe) #17

Closed
opened 2026-08-15 20:11:13 +00:00 by maximus · 0 comments
Owner

Contexte

Le chemin de lecture compare encore le token par egalite de chaine directe (index.js, checkAuth()) :

: header === `Bearer ${expected}`;

Cette comparaison s'arrete au premier octet different : le temps de reponse fuit la longueur et le prefixe du HEALTH_TOKEN. Le chemin d'ingestion passe deja par tokenMatches() (timingSafeEqual sur empreintes SHA-256, livre avec #13). Le commentaire au-dessus de tokenMatches() renvoie explicitement a cette issue.

Premisse verifiee le 2026-09-27 : le corps initial citait auth !== ; depuis le refactor de la table ROUTES (fedb1c8), la comparaison vit dans checkAuth() sous la forme header === .... Le defaut est identique.

Travail a faire

  • checkAuth() : un seul chemin pour les deux tokenKind — typeof header === "string" && header.startsWith("Bearer ") && tokenMatches(header.slice(7), expected). Seul expected varie, comme le demande la regle du point de controle unique.
  • Reecrire le commentaire de tokenMatches() (retirer « Deliberately scoped to the new write path ... issue #17 »).
  • Mettre a jour le commentaire du bloc MALFORMED dans __tests__/auth.test.js (« compared byte for byte » devient « compared in constant time via tokenMatches »).
  • Tests quasi-rates sur les routes de lecture : prefixe du token, token + suffixe, double espace apres Bearer, Bearer (espace, token vide). Tous 401.
  • Test d'epingle source : lit index.js et echoue si une comparaison directe === \Bearerreapparait (meme style que le test qui epingle leCOPYdu Dockerfile dansagent.test.js`). Un test de timing serait flaky ; l'epingle est le seul garde fiable.
  • CLAUDE.md (section Routage et auth) : tokenMatches() protege desormais les deux tokens ; retirer la mention « uniquement le chemin d'ingestion ». Mettre a jour le compte de tests.
  • Apres merge : redeploy manuel Coolify puis smoke post-deploy.

Fichiers concernes

  • index.js — checkAuth() et commentaire de tokenMatches()
  • __tests__/auth.test.js — quasi-rates + epingle source
  • CLAUDE.md — section Routage et auth, compte de tests

Surface de test

  • Existants : __tests__/auth.test.js (401 sans en-tete / mauvais token / en-tetes malformes sur les routes de lecture, fail-closed, cloisonnement des deux tokens, 404 avant 401), __tests__/state.test.js, __tests__/hosts.test.js (401 croises), test-curl.sh (niveau socket).
  • Semantique inchangee : egalite exacte avant comme apres, schema Bearer sensible a la casse. Aucun test existant ne doit bouger.
  • Regression : l'epingle source empeche un retour silencieux a la comparaison directe.

Criteres d'acceptation

  • Le controle du HEALTH_TOKEN passe par tokenMatches()
  • Les longueurs sont egalisees par hachage avant comparaison — passer les chaines brutes a timingSafeEqual fuiterait la longueur (GHSA-376v-xgjx-7mfr)
  • Les tests d'auth des routes de lecture (#12) passent inchanges ; npm test et bash test-curl.sh verts
  • Nouveaux cas quasi-rates en 401 et epingle source presente
  • Smoke post-deploy : les 6 routes de lecture (/health, /defenseurs, /defenseurs/state, /defenseurs/findings?project=..., /reports/scans?date=..., /hosts) en 200 avec token et 401 sans ; carte admin Vercel chargee

Complexite estimee

Simple — une ligne de logique, le helper existe deja.

Decisions prises

  • Planification : sortie de la vague monitoring des postes — ce changement touche TOUS les chemins de lecture en production ; le melanger aurait rendu impossible d'attribuer une casse apres le redeploy manuel.
  • Analyse (2026-09-27) : filet de test = quasi-rates + epingle source (pas de test de timing, flaky).
  • Analyse (2026-09-27) : redeploy suivi d'un smoke post-deploy sur les 6 routes de lecture.

Spec source

la-compagnie-maximus/spec-plan-monitoring-postes.md + spec-decisions-monitoring-postes.md (Issue 8)

## Contexte Le chemin de lecture compare encore le token par egalite de chaine directe (`index.js`, `checkAuth()`) : ```js : header === `Bearer ${expected}`; ``` Cette comparaison s'arrete au premier octet different : le temps de reponse fuit la longueur et le prefixe du `HEALTH_TOKEN`. Le chemin d'ingestion passe deja par `tokenMatches()` (timingSafeEqual sur empreintes SHA-256, livre avec #13). Le commentaire au-dessus de `tokenMatches()` renvoie explicitement a cette issue. Premisse verifiee le 2026-09-27 : le corps initial citait `auth !==` ; depuis le refactor de la table `ROUTES` (fedb1c8), la comparaison vit dans `checkAuth()` sous la forme `header === ...`. Le defaut est identique. ## Travail a faire - [ ] `checkAuth()` : un seul chemin pour les deux `tokenKind` — `typeof header === "string" && header.startsWith("Bearer ") && tokenMatches(header.slice(7), expected)`. Seul `expected` varie, comme le demande la regle du point de controle unique. - [ ] Reecrire le commentaire de `tokenMatches()` (retirer « Deliberately scoped to the new write path ... issue #17 »). - [ ] Mettre a jour le commentaire du bloc MALFORMED dans `__tests__/auth.test.js` (« compared byte for byte » devient « compared in constant time via tokenMatches »). - [ ] Tests quasi-rates sur les routes de lecture : prefixe du token, token + suffixe, double espace apres `Bearer`, `Bearer ` (espace, token vide). Tous 401. - [ ] Test d'epingle source : lit `index.js` et echoue si une comparaison directe `=== \`Bearer` reapparait (meme style que le test qui epingle le `COPY` du Dockerfile dans `agent.test.js`). Un test de timing serait flaky ; l'epingle est le seul garde fiable. - [ ] `CLAUDE.md` (section Routage et auth) : `tokenMatches()` protege desormais les deux tokens ; retirer la mention « uniquement le chemin d'ingestion ». Mettre a jour le compte de tests. - [ ] Apres merge : redeploy manuel Coolify puis smoke post-deploy. ## Fichiers concernes - `index.js` — `checkAuth()` et commentaire de `tokenMatches()` - `__tests__/auth.test.js` — quasi-rates + epingle source - `CLAUDE.md` — section Routage et auth, compte de tests ## Surface de test - Existants : `__tests__/auth.test.js` (401 sans en-tete / mauvais token / en-tetes malformes sur les routes de lecture, fail-closed, cloisonnement des deux tokens, 404 avant 401), `__tests__/state.test.js`, `__tests__/hosts.test.js` (401 croises), `test-curl.sh` (niveau socket). - Semantique inchangee : egalite exacte avant comme apres, schema `Bearer ` sensible a la casse. Aucun test existant ne doit bouger. - Regression : l'epingle source empeche un retour silencieux a la comparaison directe. ## Criteres d'acceptation - [ ] Le controle du `HEALTH_TOKEN` passe par `tokenMatches()` - [ ] Les longueurs sont egalisees par hachage avant comparaison — passer les chaines brutes a `timingSafeEqual` fuiterait la longueur (GHSA-376v-xgjx-7mfr) - [ ] Les tests d'auth des routes de lecture (#12) passent inchanges ; `npm test` et `bash test-curl.sh` verts - [ ] Nouveaux cas quasi-rates en 401 et epingle source presente - [ ] Smoke post-deploy : les 6 routes de lecture (`/health`, `/defenseurs`, `/defenseurs/state`, `/defenseurs/findings?project=...`, `/reports/scans?date=...`, `/hosts`) en 200 avec token et 401 sans ; carte admin Vercel chargee ## Complexite estimee Simple — une ligne de logique, le helper existe deja. ## Decisions prises - Planification : sortie de la vague monitoring des postes — ce changement touche TOUS les chemins de lecture en production ; le melanger aurait rendu impossible d'attribuer une casse apres le redeploy manuel. - Analyse (2026-09-27) : filet de test = quasi-rates + epingle source (pas de test de timing, flaky). - Analyse (2026-09-27) : redeploy suivi d'un smoke post-deploy sur les 6 routes de lecture. ## Spec source la-compagnie-maximus/spec-plan-monitoring-postes.md + spec-decisions-monitoring-postes.md (Issue 8)
maximus added the
status:ready
type:security
source:human
labels 2026-08-15 20:11:13 +00:00
maximus added
status:review
and removed
status:ready
labels 2026-09-27 20:06:19 +00:00
maximus added
status:approved
and removed
status:review
labels 2026-09-27 20:08:43 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/vps-health-api#17
No description provided.