Skip to content

RestApiConfigSchema constrains api.version with a regex the REST server never runs — the seam casts instead of parsing, so api.version: '' is accepted and mounts the whole API at /api// #11637

Description

@os-zhuang

Found while implementing #11546 (openapi info.version). Filed, not fixed — #11546's declared surface is the openapi info construction in packages/rest/src/rest-server.ts, and this is a contract-enforcement question with its own gate family.

Corrected after filing. The first version of this issue said PluginRestApiSchema.version is a bare z.string(), so '' is schema-valid. Both halves were wrong and the correction inverts the mechanism, so the whole body is restated rather than patched. There is no PluginRestApiSchema in the repo; the symbol I meant is RestApiPluginConfigSchema (packages/spec/src/api/plugin-rest-api.zod.ts:625), and it governs nothing on this path. The schema that does govern this field forbids''. The defect is that nothing enforces it. Thanks to the PM seat for catching the dangling symbol.

What was measured

On origin/main @ e3f056fc.

The governing schema, packages/spec/src/api/rest-server.zod.ts:48-52:

exportconstRestApiConfigSchema=lazySchema(()=>z.object({version: z.string().regex(/^[a-zA-Z0-9_\-\.]+$/).default('v1').describe('API version (e.g., v1, v2, 2024-01)'),

The + quantifier means '' does not match: this schema rejects an empty version, and .default('v1') only fires on undefined.

That it is the governing schema is a chain, each link measured:

  • packages/rest/src/rest-api-plugin.ts:68RestApiPluginConfig.api?: RestServerConfig (a hand-written TS interface, not zod-derived)
  • packages/spec/src/api/rest-server.zod.ts:485api: RestApiConfigSchema.optional()
  • packages/spec/src/api/rest-server.zod.ts:164RestApiConfig = z.input<typeof RestApiConfigSchema>, which is what the server casts to

The defect: declared, never enforced

Nothing parses a deployment's config against that schema. Every hop is a cast:

// packages/rest/src/rest-api-plugin.ts:388restServer=newRestServer(server,protocol,config.apiasany,/* … */);// packages/rest/src/rest-server.ts:2916constapi=(config.api??{})asPartial<RestApiConfig>;

and the guard that remains is ??, which only substitutes null/undefined:

// packages/rest/src/rest-server.ts:2924version: api.version??'v1',

Three independent confirmations that no parse happens on any deployment path:

  1. createRestApiPlugin declares no configSchema, so packages/core/src/security/plugin-config-validator.ts — which does plugin.configSchema.parse(config) when one is declared — never runs for this plugin. (git grep configSchema packages/rest/src/rest-api-plugin.ts → zero hits.)
  2. The repo's onlyRestApiConfigSchema.parse call is RestApiConfigSchema.parse({}) in packages/core/src/qa/http-adapter.ts:43, a QA helper deriving default conventions from an empty object — not deployment config.
  3. RestApiPluginConfigSchema, the sibling that does declare a bare z.string() for its own version, is referenced nowhere outside its own file, its own test, and packages/spec/api-surface/api.json. It is not the type of anything on this path, so its permissiveness is not what lets '' through either.

So the regex is a declared constraint that never executes.

Observed consequence

Driving a real RestServer with api: { version: '' }:

  • getApiBasePath() returns "/api/" (from api.apiPath ?? \${api.basePath}/${api.version}``)
  • every route mounts under it — the openapi route registers at the literal path "/api//openapi.json", with the doubled slash, and the same applies to /data, /meta, /discovery and the rest of the surface, since they all share this one base

A configuration the spec rejects is accepted by the server and produces a deployment whose every route carries a doubled slash. Which URLs that actually answers depends on the HTTP adapter's path normalization rather than on anything this repo declares.

The empty string is only the most visible instance. The regex also forbids /, whitespace and every other character outside [a-zA-Z0-9_\-\.], and none of those are refused either — version: 'v1/beta' would splice a path segment into the mount for every route.

Not prejudged

The root fix is to make this seam parse rather than cast, but where that belongs is a real call:

  • Parse in normalizeConfig() — replace the cast with RestApiConfigSchema.parse(config.api ?? {}). Declared = enforced at the point of use, and the ?? defaults become redundant because the schema's own .default() supplies them. Risk: any deployment currently booting on a config the schema rejects would start failing at boot — which is the point, but it is a behaviour change that wants measuring first.
  • Declare configSchema on the REST plugin — let the kernel's existing plugin-config-validator do it, which is the mechanism already built for this and would cover the plugin's other config too.
  • Both — the plugin validator for the authoring path, the server parse as the invariant for programmatic construction.

Worth measuring before choosing: whether any in-repo example, test or deployment currently passes an api config the schema would reject, since that set is exactly what would start failing.

Scope note

#11546 does not address this and does not depend on it. That card removed a || enriched.info.version fallback whose only trigger was this same unvalidated falsy value; it deliberately serves the configured value as written so a misconfigured deployment stays visible rather than being papered over at one of the two faces. Fixing this one would make that fallback's trigger unreachable through the authoring path, which is the correct order — the fallback should not be the thing enforcing the contract.

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions