Skip to content

v0.1.138

Choose a tag to compare

@github-actions github-actions released this 26 Aug 06:25
· 83 commits to main since this release
c285ffa

A security fix, and two doors that did not exist. A visitor holding a valid link could store a
value that made the statistics page exhaust the heap — one row was enough, it persisted, and it
fired when somebody else opened the overview. If you run an instance with tracked links, this is
the release to take.

⚠️ Operators: apply migration 0020 before or with this upgrade. It adds bounds to
commercial_doc_views and commercial_doc_sessions, repairs any out-of-range history, and only
then validates — in that order, because a validated constraint on an already-poisoned table fails.
supabase/init.sql carries the same constraints for a fresh install, and a catch-up block for a
base created before them.

Measured on the two tarballs rather than claimed: 62 files → 64, exactly two added
(server/reponses.js, supabase/migrations/0020-mesures-bornees.sql), none removed, nine
changed. All of the change is in server, cli, database and the manifest; documents,
context, types and browser are untouched — the bundle a visitor's page executes is
byte-identical across the two releases
(server/browser.generated.js, sha256 ad3af9dc56479147…,
16 809 bytes; shared.generated.js and dist/bridge.js likewise).

Fixed

  • ⚠️ Twenty JSON responses declared their type and none forbade sniffing. The JSON reply helper
    was defined thirteen times, identically, under four names (jp, jd, j, jv) across four
    route files — plus seven bodies written out by hand and five inline replies elsewhere. Found by
    measuring the text fix below, not by looking for it.
    • This is not twenty oversights. It is what a recipe becomes once it is copied: the first
      copy was correct, and it is the correction that does not propagate. nosniff was added
      repository-wide in 0.1.7; the API routes were the one place the rule stopped, because no scan
      visits them.
    • There is now one module holding the doors — server/reponses.js, depending on nothing, so
      the route families can require it without closing a cycle with handler.js. The short local
      names stay, as a single line that delegates: the convenience was legitimate, the recipe
      inside it was not. The 95 call sites do not move — a fix that rewrites 95 lines to correct
      13 reads badly and verifies worse.
    • The bodies are unchanged, byte for byte — measured, because a body that changes shape would
      change the contract hosts depend on. {"ok":false,"error":"unknown-action"} still goes out
      exactly as it did.
  • ⚠️ Three text responses left this server without the rule the repository had already written.
    The 500 at the end of /doc posted a status and a body and nothing else — no
    Content-Type at all, the one body a browser was allowed to guess. The 400 "no document
    requested" posted the type but not nosniff. And both text responses of bin/serve.js rewrote
    the recipe by hand, in the one file where the player could not post it itself. Found by the CODEX
    5.6 audit, 25/08.
    • The rule was not new: refuserEnTexte() exists precisely for this, added a month earlier when
      the first ZAP baseline scan (rule 10019) found the relay's refusals bare. A rule reapplied by
      hand is reapplied badly
      — the same lesson as the funnel bounds written in two places out of
      three. So there is now one function through which a text body leaves this server, and
      bin/serve.js calls it rather than keeping a second copy, exactly as it already does for
      POLITIQUE_PERMISSIONS.
    • ⚠️ It now survives headers that have already gone out. Its first caller is the catch of
      /doc, and an error can arrive there after sendHtml has begun writing: setHeader then
      throws ERR_HTTP_HEADERS_SENT inside the recovery itself, turning a reported error into an
      unhandled rejection. A naive fix would have introduced that. Nothing can be posted at that
      point, so it closes the stream and stays quiet — the error has already gone to
      errors.capture.
    • ⚠️ ZAP could not have seen any of this. The scan visits three served surfaces; an exception
      500 and a missing-parameter 400 are on none of them. What a scanner reaches depends on what
      it is given to visit — which is exactly why the guard below reads files instead.
  • ⚠️ The [Unreleased] section had been written twice. Two branches each opened their own,
    git merged both without a conflict — the file then carried two identical titles, and
    sectionDe() (which the release preflight uses to extract a version's notes) stops at the first.
    Half the notes would have shipped missing, silently. The two sections are merged here, and
    the guard now refuses any repeated section title: this was invisible to it because it only
    ever read version numbers and the footer link, never the headings themselves.
  • ⚠️ A stored denial of service in the analytics funnel. A visitor holding a valid link could
    post {"event":"page","page":2147483647,"maxPage":2147483647}. logView() checked only that the
    number was finite, the integer column accepted it, and the overview's funnel then looped from 1
    to that value — measured: FATAL ERROR: JavaScript heap out of memory in eight seconds, on a
    single stored row, with the process capped at 512 MB. One row was enough, it persisted, and the
    trigger was someone else opening the statistics, later. Found by the CODEX 5.6 audit on
    v0.1.137, reproduced here before being believed.
    • The bounds already existed 275 lines below, added against exactly this class of defect on
      the two session paths. logView was the third path, missed when the other two were closed.
      So the fix is not "bound here too": there is now one function through which a measurement
      enters the database, and one through which a page is read back. There is no second place left
      to forget. (AGENTS.md, the rule written while fixing this: you do not check the crossing, you
      remove it.)
    • The database is no longer assumed clean. Bounding writes protects future rows; the ones
      already stored remain. Every read of a page value is clamped, including the ones that are only
      displayed — "this reader reached page 2 147 483 647" is a false number served to a human who
      decides.
    • The funnel is now O(pages + sessions) — histogram plus descending cumulative — instead of
      rescanning every session per page. Equivalence with the previous implementation checked on
      3 000 random draws and six edge cases. On legitimate values it also matters: 10 000 pages ×
      400 sessions was four million comparisons for a result two passes give exactly.
    • Migration 0020 adds CHECK constraints on both tables, NOT VALID first, then repairs
      out-of-range history, then validates — in that order, because a validated constraint on an
      already-poisoned table fails, and a migration that fails on the data it came to repair is one
      an operator stops running. Mirrored into supabase/init.sql.
  • ⚠️ Nine route failures returned 500 without reporting anything. The body of bot-tts — and
    eight sibling routes across routes-agent, routes-direct and routes-liens — was wrapped in a
    bare catch that returned { ok: false }: no stack, no message, no call to errors.capture.
    That is why the crypto defect fixed in 0.1.137 lived through two releases: a host's
    monitoring could not have seen it even correctly wired
    — the route was silent, not their
    instrument. It was found by reading the code, not by watching it.
    • It was a repeated omission, not a doctrine: handler.js, presentations.js and retention.js
      have captured for a long time, and exactly one of the ten route catches did. All nine now
      report the error and name the request's actual action before returning 500 — each catch
      covers a block of actions (the one in routes-agent covers eight), so a fixed label would
      have lied on eight calls out of nine.
    • All nine paths are covered by a bench, not just the one that broke. The first version covered
      bot-tts alone and said so; CI refused it on coverage, and was right — nine silent failures
      replaced by nine untested reporting paths is the same fault, smaller. Statement coverage goes
      from 90.31% to 90.81%.

Added

  • A guard that refuses a hand-written text body sent without its type and nosniff. It reads an
    AST, not a pattern: this repository has paid three times for regex guards — uses:, FROM,
    and crypto, where the last one accused the very file it had just had fixed.
    • A second rule closes the JSON door rather than counting oversights. The thirteen copies all
      sent a computed body (res.end(JSON.stringify(obj))), which the first rule does not look at
      and should not. What they had in common was declaring the type — each deciding, on its own,
      what accompanies that declaration. So outside server/reponses.js, no file may declare
      application/json. A fourteenth copy is refused. Other types (text/html, text/javascript,
      application/pdf) are deliberately outside this rule and the guard's header says so: they have
      their own senders, which already set nosniff.
    • ⚠️ Its first version was wrong, and measuring said so. It flagged every string literal and
      accused seven res.end('{"ok":…}') in routes-liens.js — seven JSON bodies that post their
      type, line by line. Seven false accusations out of seven findings, the same failure as the
      docker run pattern the day before. Corrected in the guard, not in the correct code: the rule
      is the one written by hand three times — a body in text (type absent, or text/…) must
      forbid sniffing
      . A JSON body declaring application/json is not that fault.
  • The registry images CI pulls are pinned by digest. postgres:16-alpine (the real-database
    benches) and postgrest/postgrest:v12.2.3 ran on moving tags: two runs of the same commit
    could prove different things, and a version tag is a publisher's convention, not a property of
    the registry. Both now carry an @sha256: digest with the tag kept beside it, exactly as the
    Dockerfile rule has required for weeks (CODEX 5.6 audit, 25/08).
    • ⚠️ A registry image is now declared, never written inside a command — in
      services.<name>.image, container:, or an env: variable named IMAGE_…. The first version
      of the guard read run: blocks with a pattern and accused three places: host (taken out of
      --network host) and two images built locally, which have no registry and nothing to pin —
      three false accusations out of three findings. A run: block is shell; telling an image from
      an argument there means re-implementing docker run's grammar. So the boundary was removed
      rather than checked, and tools/images-des-workflows.mjs reads YAML only. It also refuses a
      $IMAGE_… used without a declaration, so the convention does not rest on remembering it.
    • dumb-init in the Dockerfile is still unpinned and is named here rather than quietly
      left out: the Alpine package index is unreachable from the environment this was prepared in
      (the egress proxy answers 403), so the version could not be resolved — and inventing one would
      be worse than the gap.
  • The schema parity check now compares constraints. CI proved that init.sql carries every
    column, index, function, trigger, publication and replica identity the migrations bring — but not
    their constraints, so 0020's could have drifted between the two files unnoticed. Same blind spot
    already closed twice, for nullability and for triggers.
  • The migration guard reads DO blocks, which it used to strip as function bodies. A DO
    block is not a body someone calls later — it is code the migration executes, so what it
    declares, the migration declares. 0020 puts its six constraints in one (there is no
    add constraint if not exists in PostgreSQL) and was accused of leaving no probeable sign. And
    add constraint <name> is now itself a sign: pg_constraint answers for it.
  • A guard that refuses a Node-only crypto method called on the global. crypto has been a
    global since Node 18, so no-undef cannot help: the variable exists, it just means something
    else — WebCrypto, which has no createHash. The list of module-only methods is derived from
    the running Node
    (63 of 65 on Node 22; only getRandomValues and randomUUID live on both
    sides), never typed by hand.
    • ⚠️ It reads the code with a real lexer. Three regex versions were written first, and all
      three were blind — the last one was defeated by a file-glob quoted inside a line comment,
      whose slash-star opened a block the pattern closed twenty-four lines later, swallowing the very
      require whose absence it was hunting. It accused the fixed file. This repository has paid for
      that lesson twice before, on uses: and on FROM.

What changed in the package, by zone — 0.1.1370.1.138

Measured on the two tarballs, by the release workflow. Not written by hand.

Zone What it is Added Removed Changed
documents what a human reads 0 0 0
manifest package.json — version, exports, dependencies 0 0 1
server the code the host executes 1 0 6
context the injected-context implementations 0 0 0
browser-types the declarations the host's tsc reads for « ./bridge » — breaks a build, never a page 0 0 0
browser what the visitors' page executes 0 0 0
cli the command-line entry point 0 0 1
types declarations for the server and context entry points — breaks a build, never runtime 0 0 0
database the schema and the migrations the host applies itself 1 0 1
The files themselves
~ package.json
+ server/reponses.js
~ server/handler.js
~ server/routes-agent.js
~ server/routes-direct.js
~ server/routes-liens.js
~ server/routes-visiteur.js
~ server/shares.js
~ bin/serve.js
+ supabase/migrations/0020-mesures-bornees.sql
~ supabase/init.sql

⚠️ database — 1 file(s) changed, not added

a migration already applied elsewhere must be immutable. A changed file here is not a count — it is a stop. Check the objects listed below against your live database: a migration registry only records what went through one particular path, so absence from it does not mean the migration was never applied.

  • supabase/init.sql
    objects it touches — table public.commercial_doc_shares — create, alter, index cds_doc_id_idx — create, index cds_parent_idx — create, index idx_doc_shares_brand_key — create, index idx_shares_revoked_at — create, table public.commercial_doc_views — create, alter, index cdv_slug_idx — create, index cdv_doc_idx — create, index commercial_doc_views_at_idx — create, table public.commercial_doc_sessions — create, alter, index cds_sess_slug_idx — create, index cds_sess_doc_idx — create, index idx_cds_last_at — create, table public.commercial_doc_internal_sessions — create, alter, index cdis_doc_idx — create, index commercial_doc_internal_sessions_last_at_idx — create, table public.doc_presentations — create, alter, index doc_presentations_active_idx — create, index doc_presentations_last_seen_idx — create, index doc_presentations_owner_active_idx — create, index doc_presentations_owner_email_active_idx — create, table public.doc_presentation_messages — create, alter, index dpm_slug_idx — create, function public.player_bump_message_seq — create, trigger dpm_bump_modseq — drop, create, index idx_dpm_slug_modseq — create, table public.doc_presentation_attendees — create, alter, index doc_presentation_attendees_slug_idx — create, index idx_attendees_slug_creator — create, table public.doc_bot_sessions — create, alter, index idx_bot_last_at — create, index doc_bot_sessions_share_idx — create, table public.player_rate_limits — create, alter, index player_rate_limits_expires_idx — create, function public.player_rate_limit_bump — create, function public.player_attendance_bump — drop, create, function public.player_archive_scellee — create, trigger dpm_archive_scellee — drop, create, trigger dpa_archive_scellee — drop, create, table public. — alter

    @@ -93,9 +93,19 @@
       doc_id          text,
       recipient_email text,
       event           text not null,             -- open | page | heartbeat
    -  page            integer,
    -  max_page        integer,
    -  seconds         integer,
    +  -- ⚠️ BORNES EN BASE, PARCE QUE LE CODE N'EST PAS LE DERNIER REMPART (0020, audit CODEX 5.6).
    +  -- ⚠️ ET NOMMÉES, COMME DANS LA MIGRATION. Un `check` anonyme reçoit un nom AUTOMATIQUE de
    +  -- PostgreSQL : une base née d'`init.sql` et une base migrée porteraient alors la même règle sous
    +  -- deux noms, et rejouer les migrations par-dessus en ajouterait six de plus. La parité de schéma
    +  -- de la forge a refusé exactement ça — dans la PR qui venait de lui apprendre à regarder les
    +  -- contraintes. Un hôte prouve le passage de la 0020 en demandant `ck_views_page_borne` ; le nom
    +  -- EST le signe, il ne peut donc pas être laissé au hasard.
    +  -- Ces trois colonnes sont peuplées par le PUBLIC. Sans plage, un visiteur muni d'un lien valide
    +  -- y écrivait 2147483647 ; l'agrégation du funnel bouclait ensuite jusque-là, à l'ouverture des
    +  -- statistiques — chez quelqu'un d'autorisé, plus tard, sur une ligne posée une seule fois.
    +  page            integer constraint ck_views_page_borne check (page is null or (page >= 0 and page <= 10000)),
    +  max_page        integer constraint ck_views_max_page_borne check (max_page is null or (max_page >= 0 and max_page <= 10000)),
    +  seconds         integer constraint ck_views_seconds_borne check (seconds is null or (seconds >= 0 and seconds <= 86400)),
       session_id      text,
       at              timestamptz not null default now(),
       ua              text
    @@ -113,9 +123,10 @@
       slug            text not null,
       doc_id          text,
       recipient_email text,
    -  num_pages       integer,
    -  max_page        integer,
    -  total_seconds   integer default 0,
    +  -- Mêmes bornes que `commercial_doc_views`, même raison : ces trois-là viennent aussi du dehors.
    +  num_pages       integer constraint ck_sessions_num_pages_borne check (num_pages is null or (num_pages >= 0 and num_pages <= 10000)),
    +  max_page        integer constraint ck_sessions_max_page_borne check (max_page is null or (max_page >= 0 and max_page <= 10000)),
    +  total_seconds   integer default 0 constraint ck_sessions_total_seconds_borne check (total_seconds is null or (total_seconds >= 0 and total_seconds <= 86400)),
       pages_time      jsonb   default '{}'::jsonb,
       ua              text,
       ip              text,
    @@ -652,6 +663,41 @@
     alter table public.commercial_doc_shares
       add column if not exists revoked_at timestamptz;
     update public.commercial_doc_shares set revoked_at = now() where revoked = true and revoked_at is null;
    +
    +-- ⚠️ ET LE RATTRAPAGE VAUT AUSSI POUR LES CONTRAINTES, PAS SEULEMENT POUR LES COLONNES (0020).
    +-- Elles sont déclarées dans le corps des tables ci-dessus, ce qui règle la base VIERGE — et ne
    +-- touche pas une base déjà installée, où `create table if not exists` ne fait rien. Le scénario
    +-- « init v0.1.64 → rejeu de l'init actuel » a refusé exactement ça : la base neuve portait les six
    +-- bornes, la base rattrapée non, et les deux formes divergeaient en silence.
    +--
    +-- Même ordre que la migration, et pour la même raison : `not valid`, on répare, puis on valide.
    +-- Une contrainte validée d'emblée échoue sur une table déjà empoisonnée, et une réparation qui
    +-- échoue sur la donnée qu'elle vient réparer ne se relance plus.
    +do $$
    +declare
    +  c record;
    +begin
    +  for c in
    +    select * from (values
    +      ('commercial_doc_views',    'ck_views_page_borne',             'page',          10000),
    +      ('commercial_doc_views',    'ck_views_max_page_borne',         'max_page',      10000),
    +      ('commercial_doc_views',    'ck_views_seconds_borne',          'seconds',       86400),
    +      ('commercial_doc_sessions', 'ck_sessions_max_page_borne',      'max_page',      10000),
    +      ('commercial_doc_sessions', 'ck_sessions_num_pages_borne',     'num_pages',     10000),
    +      ('commercial_doc_sessions', 'ck_sessions_total_seconds_borne', 'total_seconds', 86400)
    +    ) as t(tab, nom, col, maxi)
    +  loop
    +    if not exists (select 1 from pg_constraint where conname = c.nom) then
    +      execute format(
    +        'alter table public.%I add constraint %I check (%I is null or (%I >= 0 and %I <= %s)) not valid',
    +        c.tab, c.nom, c.col, c.col, c.col, c.maxi);
    +    end if;
    +    execute format(
    +      'update public.%I set %I = least(greatest(%I, 0), %s) where %I is not null and (%I < 0 or %I > %s)',
    +      c.tab, c.col, c.col, c.maxi, c.col, c.col, c.col, c.maxi);
    +    execute format('alter table public.%I validate constraint %I', c.tab, c.nom);
    +  end loop;
    +end $$;
     -- Une base née d'un init.sql d'avant la 0008 porte un NOT NULL qui rend la clôture impossible.
     alter table public.doc_presentations
       alter column control_hash drop not null;