🐛 Express catch-all 404 covers GET only — other verbs fall through to Express's default HTML

[PierreBrisorgueil]

Problem

#3975 replaced the implicit-200 catch-all with a content-negotiated 404. The handler is registered on app.get only:

// lib/services/express.js — initModulesServerRoutes
app.get('/{*path}', (req, res) => {
  const isApiPath = /^\/api(\/|$)/i.test(req.path);
  if (isApiPath || req.accepts(['html', 'json']) === 'json') {
    return res.status(404).json({ error: 'not_found' });
  }
  res.status(404).send('<center><br /><h1>404</h1><h3>Not Found</h3></center>');
});

initModulesServerRoutes ends there, and initErrorRoutes only registers a 4-arity error handler, which fires on next(err) and never on an unmatched route. So nothing handles unmatched POST / PUT / PATCH / DELETE: they fall through to Express's default finalhandler and return HTML (Cannot POST /api/…).

That contradicts the stated intent of the change, quoted from its own comment:

API consumers and automated agents get a proper JSON error instead of a false-positive success.

An API client gets JSON on a bad GET and HTML on a bad POST.

Not theoretical

MIGRATIONS.md documents removed routes that are POSTs — POST /api/organizations/:organizationId/invites, POST /api/invites/:token/accept. A client still calling those is exactly the audience #3975 targeted, and it currently receives HTML.

Why it slipped

The nine tests added in lib/services/tests/express.notfound.unit.tests.js are all GET. No verb coverage.

Proposed fix

• Register the catch-all with app.all instead of app.get, so every unmatched verb gets the same content-negotiated 404. The explicit app.get('/') root route stays registered before it and is unaffected.
• Add tests for POST / PUT / PATCH / DELETE on both /api/* and non-API paths.
• Check OPTIONS explicitly. app.all would also match preflight requests. The cors middleware normally answers preflight before routes are reached, but that ordering needs a test rather than an assumption — it is the one real risk in this change.

Second gap from the same change: no migration note

#3975 is a behavioural break for any consumer that treated a 200 on an unknown path as success, and it shipped with no MIGRATIONS.md entry. A short note would help anyone whose readiness/health check happens to poll a path that is not a declared route — that check silently flips from passing to a hard timeout, while the server is perfectly healthy.

• Add a MIGRATIONS.md entry: unmatched paths no longer return 200; health checks must target a declared route such as /api/health.

Worth noting this repo's own CI already polls /api/health, so it could not observe the regression it introduced.

Small enough to land as a single PR.