diff --git a/.changeset/openapi-info-version-is-the-api-version.md b/.changeset/openapi-info-version-is-the-api-version.md new file mode 100644 index 0000000000..57868de115 --- /dev/null +++ b/.changeset/openapi-info-version-is-the-api-version.md @@ -0,0 +1,28 @@ +--- +'@objectstack/rest': patch +--- + +`GET {basePath}/openapi.json` no longer falls back to the spec package's +compile-time version when `api.version` is configured empty + +The served `info.version` has always carried the API version identifier +(`api.version`, default `'v1'`), under a comment claiming it carried "the +runtime version so consumers don't pin to the spec package's compile-time +version". Both halves were false: the runtime version never reached the field, +and the `|| enriched.info.version` fallback published exactly the compile-time +version the comment said the line existed to avoid. + +The fallback was reachable rather than dead, though not because the contract +permits it: `RestApiConfigSchema` declares +`version: z.string().regex(/^[a-zA-Z0-9_\-\.]+$/)`, which refuses `''`. Nothing +parses this config against that schema — both hops into the server are casts — +so `normalizeConfig`'s `??` is the only guard, it does not catch `''`, and the +document advertised `@objectstack/spec`'s package version. It now serves the configured value as +written, so a misconfigured deployment stays visibly misconfigured instead of +silently switching the field to a different kind of fact. Every non-empty +`api.version` — including the default — serves exactly what it served before. + +`info.version` is deliberately not the runtime version: OpenAPI 3.1 defines it +as "the version of the OpenAPI document (which is distinct from the OpenAPI +Specification version or the API implementation version)". Callers who want the +serving artifact read `{basePath}/discovery` or `/health`. diff --git a/packages/rest/src/rest-openapi-route.test.ts b/packages/rest/src/rest-openapi-route.test.ts index 59cabf3026..6957628657 100644 --- a/packages/rest/src/rest-openapi-route.test.ts +++ b/packages/rest/src/rest-openapi-route.test.ts @@ -369,3 +369,84 @@ describe('#5588 — built-in routes come from rest, not from the static artifact } }); }); + +describe('GET /openapi.json — what `info.version` carries (#11546)', () => { + // The line under test used to read + // `version: this.config.api.version || enriched.info.version` + // under a comment promising "the runtime version so consumers don't pin to + // the spec package's compile-time version". Both halves were false, and + // nothing pinned either one, so the document could have drifted to any of + // three different facts without a test noticing. These four fix what the + // field means. + // + // OpenAPI 3.1, Info Object: `version` is "the version of the OpenAPI + // document (which is distinct from the OpenAPI Specification version or the + // API implementation version)". The runtime version is the implementation + // version, so it is the one value the field's own definition excludes — + // which is why this is NOT the shape #11292 settled for `/discovery`, where + // `DiscoverySchema.version` means the serving artifact by #10993. + + it('serves the declared API version identifier, not the artifact version', async () => { + const rest = makeRest(makeProtocol({ object: [], api: [] }).protocol); + const artifact = await (rest as any).loadOpenApiSpec(); + const { body } = await serveOpenApiFrom(rest); + + expect(body.info.version).toBe('v1'); + // The serve path deliberately overrides the producer here, so the pin is + // only meaningful while the two values actually differ — if they ever + // converge this assertion says so instead of passing vacuously. + expect( + artifact.info.version, + 'the artifact must carry a DIFFERENT version for the override pin above to mean anything', + ).not.toBe('v1'); + expect(body.info.version).not.toBe(artifact.info.version); + }); + + it('tracks a custom `api.version`, which is also the mount segment', async () => { + const rest = makeRest(makeProtocol({ object: [], api: [] }).protocol, { version: 'v9' }); + const { body } = await serveOpenApiFrom(rest, '/api/v9'); + expect(body.info.version).toBe('v9'); + }); + + it('is not the runtime version — an `OS_RUNTIME_VERSION` stamp does not reach it', async () => { + // The anti-regression pin for the direction this card did NOT take. Were + // the field re-pointed at `resolveDiscoveryVersion()`, the sentinel below + // would land in the served document and this goes red. + const SENTINEL = '9.9.9-openapi-info-version-sentinel'; + const old = process.env.OS_RUNTIME_VERSION; + process.env.OS_RUNTIME_VERSION = SENTINEL; + try { + const rest = makeRest(makeProtocol({ object: [], api: [] }).protocol); + const { body } = await serveOpenApiFrom(rest); + expect(body.info.version).toBe('v1'); + expect(JSON.stringify(body.info)).not.toContain(SENTINEL); + } finally { + if (old === undefined) delete process.env.OS_RUNTIME_VERSION; + else process.env.OS_RUNTIME_VERSION = old; + } + }); + + it('serves a falsy `api.version` as itself rather than falling back to the artifact', async () => { + // The removed `|| enriched.info.version` was reachable, not dead — though + // not because the contract permits `''`. `RestApiConfigSchema` refuses it + // (`z.string().regex(/^[a-zA-Z0-9_\-\.]+$/)`); nothing parses this config + // against that schema, so `??` is the only guard and `''` walks past it. + // Measured on the pre-fix code this served the spec package's compile-time + // version — the exact value the old comment said the line existed to keep + // off the wire. + // + // An empty version is a broken deployment either way (the mount doubles its + // slash, below). The point of the pin is that it stays visibly broken + // instead of quietly publishing a different kind of fact. + const rest = makeRest(makeProtocol({ object: [], api: [] }).protocol, { version: '' }); + expect( + (rest as any).getApiBasePath(), + 'this pin describes the empty-version mount — if normalization starts rejecting it, retire the pin', + ).toBe('/api/'); + + const artifact = await (rest as any).loadOpenApiSpec(); + const { body } = await serveOpenApiFrom(rest, '/api/'); + expect(body.info.version).toBe(''); + expect(body.info.version).not.toBe(artifact.info.version); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 95c26ffd28..a1e41dfee9 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -3619,12 +3619,50 @@ export class RestServer { logError('[REST] openapi.json endpoint enrichment skipped:', err?.message ?? err); } - // Surface the runtime version so consumers don't pin to - // the spec package's compile-time version. + // `info.version` carries the API version identifier this + // deployment declares (`api.version`, which `normalizeConfig` + // defaults to `'v1'`) — the same value that builds the default + // mount (`${basePath}/${version}` -> `/api/v1`), though a + // deployment that sets `apiPath` moves the mount without + // moving this. + // + // It is deliberately NOT the runtime version, and the comment + // this replaces ("surface the runtime version so consumers + // don't pin to the spec package's compile-time version") had it + // backwards in both halves. OpenAPI 3.1 defines the field as + // "the version of the OpenAPI document (which is distinct from + // the OpenAPI Specification version or the API implementation + // version)" — the runtime version IS the implementation + // version, the one reading the field's own definition rules + // out. That fact is not lost: `{basePath}/discovery` and + // `/health` both answer with it, derived from + // `OS_RUNTIME_VERSION` (#10993/#11235/#11292), so a caller who + // wants the serving artifact asks a producer that means it. + // + // No `|| enriched.info.version` fallback. It was reachable, + // not dead — and NOT because the contract allows an empty + // version. `RestApiConfigSchema` declares + // `version: z.string().regex(/^[a-zA-Z0-9_\-\.]+$/)`, which + // refuses `''`. Nothing ever runs it: this config arrives + // through casts on both hops (`config.api as any` in + // `rest-api-plugin.ts`, then `as Partial` in + // `normalizeConfig` below), the plugin declares no + // `configSchema` for the kernel's validator to parse, and the + // repo's only `RestApiConfigSchema.parse` parses `{}` in a QA + // helper. So the regex never executes on a deployment path, + // `??` is the only guard left, and `''` walks past it — + // whereupon the fallback published the spec package's + // compile-time version, the one value the old comment claimed + // this line existed to keep off the wire. A falsy + // `api.version` now serves itself, so a misconfigured + // deployment reads as misconfigured instead of silently + // switching this field to a different kind of fact. The + // unenforced regex is a defect in its own right, filed + // separately rather than fixed here. if (enriched.info) { enriched.info = { ...enriched.info, - version: this.config.api.version || enriched.info.version, + version: this.config.api.version, }; }