fix(admin): gate /admin/openapi.json behind a feature flag (#7693)

Address Qodo finding 1: new features must ship behind a flag, disabled
by default (CONTRIBUTING.md, AGENTS.MD, best_practices.md). Adds
settings.adminOpenAPI.enabled (default false). expressPreSession
returns early when the flag is off, so the route is dormant on a fresh
install.

The codegen pipeline imports generateAdminDefinition() in-process and
does not depend on the runtime route, so default-off has no effect on
the typed client. Operators who want third-party tooling (Postman,
swagger-ui, downstream clients) to consume the spec at runtime opt in
via settings.

Adds:
- SettingsType + defaults entry in src/node/utils/Settings.ts
- settings.json.template documentation
- A backend spec asserting expressPreSession is a no-op when the flag
  is off (live route test now sets the flag explicitly)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-08 16:32:35 +01:00
parent 2c91b75d18
commit dbcfb39027
4 changed files with 72 additions and 3 deletions

View file

@ -230,6 +230,19 @@
"requireAdminForStatus": false
},
/*
* Admin OpenAPI document at /admin/openapi.json.
*
* Disabled by default per Etherpad's "new features behind a flag, off by
* default" policy. The admin UI's typed client embeds the spec at build
* time, so the runtime route is only useful for third-party tooling
* (Postman, swagger-ui, downstream clients). Set `enabled: true` to mount
* the route; the SPA at /admin/ continues to serve normally either way.
*/
"adminOpenAPI": {
"enabled": false
},
/*
* Contact address for admin notifications (updates, security advisories, future features).
* Set to null to disable outbound mail from the updater.

View file

@ -1,7 +1,7 @@
'use strict';
import {ArgsExpressType} from '../../types/ArgsExpressType';
import {getEpVersion} from '../../utils/Settings';
import settings, {getEpVersion} from '../../utils/Settings';
const OPENAPI_VERSION = '3.0.2';
@ -11,8 +11,8 @@ const OPENAPI_VERSION = '3.0.2';
* Distinct from the public versioned API document built by openapi.ts
* admin routes are plain Express handlers (not APIHandler-driven), so this
* spec is hand-authored. The shape is consumed by admin/scripts/dump-spec.ts
* for client-side codegen and exposed at GET /admin/openapi.json for
* downstream tooling.
* for client-side codegen, and (when settings.adminOpenAPI.enabled) exposed
* at GET /admin/openapi.json for downstream tooling.
*/
export const generateAdminDefinition = (): any => ({
openapi: OPENAPI_VERSION,
@ -159,6 +159,11 @@ export const expressPreSession = async (
_hookName: string,
{app}: ArgsExpressType,
): Promise<void> => {
// Behind a feature flag, default off. Etherpad policy
// (CONTRIBUTING.md, AGENTS.MD) requires new features to ship disabled by
// default. The route is only useful for third-party tooling — codegen
// imports generateAdminDefinition() in-process and does not depend on it.
if (!settings.adminOpenAPI?.enabled) return;
app.get('/admin/openapi.json', (_req: any, res: any) => {
res.header('Access-Control-Allow-Origin', '*');
res.json(generateAdminDefinition());

View file

@ -331,6 +331,9 @@ export type SettingsType = {
githubRepo: string,
requireAdminForStatus: boolean,
},
adminOpenAPI: {
enabled: boolean,
},
adminEmail: string | null,
getPublicSettings: () => Pick<SettingsType, "title" | "skinVariants"|"randomVersionString"|"skinName"|"toolbar"| "exposeVersion"| "gitVersion" | "enablePadWideSettings" | "privacyBanner">,
}
@ -510,6 +513,18 @@ const settings: SettingsType = {
// disabling the updater itself.
requireAdminForStatus: false,
},
/**
* Admin OpenAPI document endpoint at /admin/openapi.json.
*
* Disabled by default per Etherpad's "new features behind a flag, off by
* default" policy (see CONTRIBUTING.md). The codegen pipeline imports
* generateAdminDefinition() in-process and does not depend on the route;
* enable this only if you want third-party tooling (Postman, swagger-ui,
* downstream clients) to consume the spec at runtime.
*/
adminOpenAPI: {
enabled: false,
},
/**
* Contact address for admin notifications (updates, future security advisories).
* Null disables outbound mail from the updater.

View file

@ -171,8 +171,16 @@ describe('admin OpenAPI document', function () {
});
describe('GET /admin/openapi.json', function () {
// The route is feature-flagged (settings.adminOpenAPI.enabled, default
// false). expressPreSession reads the flag once at registration time, so
// we set it before common.init() boots Express. Mocha runs this `before`
// hook prior to any inner `it`, and it runs before the default-off
// describe below sees common.init().
let agent: any;
before(async function () {
const settings = require('../../../node/utils/Settings').default;
settings.adminOpenAPI = settings.adminOpenAPI || {enabled: true};
settings.adminOpenAPI.enabled = true;
const common = require('../common');
agent = await common.init();
});
@ -191,4 +199,32 @@ describe('admin OpenAPI document', function () {
assert.equal(res.headers['access-control-allow-origin'], '*');
});
});
describe('feature-flag default-off behavior', function () {
it('expressPreSession is a no-op when settings.adminOpenAPI.enabled is false', async function () {
// Boot a stub express app, run expressPreSession with the flag off,
// and assert no GET /admin/openapi.json route was registered. We
// assert on the spy directly because the live server in the previous
// describe has the flag forced on.
const registered: string[] = [];
const stubApp: any = {
get: (path: string, _h: any) => {
registered.push(path);
},
};
const settingsModule = require('../../../node/utils/Settings').default;
const prev = settingsModule.adminOpenAPI?.enabled;
try {
settingsModule.adminOpenAPI = {enabled: false};
await openapiAdmin.expressPreSession('expressPreSession', {app: stubApp});
assert.equal(
registered.includes('/admin/openapi.json'),
false,
'route should not be registered when flag is off',
);
} finally {
if (settingsModule.adminOpenAPI) settingsModule.adminOpenAPI.enabled = !!prev;
}
});
});
});