Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/openapi-info-version-is-the-api-version.md
Original file line numberDiff line numberDiff line change
@@ -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`.
81 changes: 81 additions & 0 deletions packages/rest/src/rest-openapi-route.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
});
});
44 changes: 41 additions & 3 deletions packages/rest/src/rest-server.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<RestApiConfig>` 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,
};
}

Expand Down
Loading