Skip to content

Remove the internal route option; route CLI bootstrap (register-super, mail-test) through the app directly #70

Description

@taylortom

Summary

Remove the internal route option from the framework. It was intended to restrict routes to localhost, but the check (ServerUtils.handleInternalRoutes) reads req.ip, which is unreliable behind a production nginx reverse proxy: with trustProxy off, every proxied request appears as 127.0.0.1 (so the check passes for everyone); with it on, the result is X-Forwarded-For-derived and potentially spoofable. So internal does not reliably do what it claims.

Only three routes use it, and all can stop relying on it:

Module Route Current guard Plan
adapt-authoring-auth-local POST /registersuper internal only (no auth) Delete route; bootstrap via the app directly
adapt-authoring-mailer POST /test internal only (no auth) Delete route; call the app directly
Metrics endpoint GET /metrics internal + read:metrics scope Keep scope; drop internal

The two no-auth routes exist solely so the at-utils CLI commands (register-super, mail-test) can reach them over loopback without credentials. at-utils already boots the app itself (startAppApp.instance.onReady()), and the underlying logic is already exposed as public module methods (LocalAuthModule.registerSuper(data), MailerModule.send(...)). So the CLIs can call the modules directly and the routes — and internal — become unnecessary.

⚠️ Note: because internal is a no-op behind nginx, /registersuper and mailer/test may currently be reachable unauthenticated in production. /registersuper self-disables once a super user exists (limited blast radius); mailer/test could be an open send-email endpoint. Worth verifying/patching at nginx independently of this work.

Plan

Ordered so no route is ever left exposed.

1. adapt-authoring-server — add a headless boot mode

startApp runs full onReady(), which makes the server listen(). at-utils only boots today when no server is running, so there is no conflict; once the CLIs always boot, mail-test against a live server would EADDRINUSE. Add a no-listen flag (skip listen() when e.g. app.args['no-serve'] is set). Small and reusable for any admin CLI.

2. at-utils — call modules directly

  • register-super: startApp (headless) → (await App.instance.waitForModule('auth-local')).registerSuper({ email, password }). Remove the internalApiRequest ping/HTTP path.
  • mail-test: startApp (headless) → mailer.send({ to, subject, text }) (or a small sendTestEmail() wrapper).

3. Delete the routes

  • Remove POST /registersuper from adapt-authoring-auth-local (keep registerSuper).
  • Remove POST /test from adapt-authoring-mailer (keep send).

4. Drop internal from the metrics route

read:metrics is the guard; localhost-only, if wanted, is an nginx location block (documented in the module's docs/metrics.md). Any other downstream module declaring internal: true must do the same before step 5.

5. adapt-authoring-server — remove the mechanism (last)

Delete handleInternalRoutes (ServerUtils.js), its middleware entry (Router.js:61), and the internal property from schema/routeitem.schema.json. Update server tests/fixtures. Breaking change → major version bump.

Sequencing

Ship steps 1–4 first so every consumer stops using internal; bump the umbrella to the new module versions; then land step 5. Removing the mechanism while any route still declares internal: true would silently make that route public.

Repos / PRs

  • adapt-authoring-server (headless mode, then mechanism removal — two PRs / releases)
  • at-utils
  • adapt-authoring-auth-local
  • adapt-authoring-mailer
  • any module exposing the metrics route (drop internal)
  • umbrella: bump dependencies once the above are released

Acceptance criteria

  • register-super and mail-test work with no server running and against a running server, without HTTP/loopback calls.
  • /registersuper and mailer/test routes removed; registerSuper / send methods retained.
  • No route in the codebase declares internal: true.
  • internal property, handleInternalRoutes, and its middleware removed from adapt-authoring-server; tests updated.
  • Production nginx exposure of the (now-removed) routes verified/closed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    New

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions