mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-18 09:04:54 +00:00
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:
parent
2c91b75d18
commit
dbcfb39027
4 changed files with 72 additions and 3 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue