Skip to content

openapi3: return a typed error for a disallowed external $ref - #27

Open
reuvenharrison wants to merge 21 commits into
masterfrom
feat/typed-disallowed-external-ref
Open

reuvenharrison wants to merge 21 commits into
masterfrom
feat/typed-disallowed-external-ref

Conversation

@reuvenharrison

@reuvenharrison reuvenharrison commented Oct 22, 2026 •

Copy link
Copy Markdown

The loader refuses a $ref to another document when IsExternalRefsAllowed is false, and reported it with fmt.Errorf, so the message was the only thing distinguishing a refusal from a document that failed to load for another reason. This returns *DisallowedExternalRefError instead, carrying the offending $ref.

The message text is unchanged, so a consumer matching on it keeps working.

Upstream: getkin#1370

🤖 Generated with Claude Code

https://claude.ai/code/session_017e285CvofYf6joqC98gTXz

@reuvenharrison
reuvenharrison force-pushed the feat/typed-disallowed-external-ref branch 2 times, most recently from 6dcd073 to a74f267 Compare October 22, 2026 08:33
fenollp and others added 21 commits October 22, 2026 13:00
…y version

The Reference Object of OpenAPI 3.0, 3.1 and 3.2 alike says "any
properties added SHALL be ignored". From 3.1 on only summary and
description mean something there, and the loader already applies them;
anything else was still reported as extra sibling fields, failing
documents the specification calls valid.

So validateExtras now only reports the x- extensions next to a $ref, under
ProhibitExtensionsWithRef. That option was disabled for OpenAPI 3.1
schemas, only because their sibling keywords were reported together with
those extensions: it now applies in every version, as documented.
ExtraSiblingFieldsError is still returned for the unknown fields of inline
objects.

validateExtras also reported the fields next to a $ref once the $ref was
gone: InternalizeRefs replaces a component that is a $ref by what it
refers to, which leaves those fields out of the document, but not out of
what Validate checked.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
… schema

In JSON Schema 2020-12, on which OpenAPI 3.1 bases its Schema Object,
the keywords next to a $ref apply alongside it: an instance must satisfy
both the referenced schema and its siblings. They were instead copied
over the referenced schema's keywords of the same name, so a sibling
maxLength: 10 relaxed a referenced maxLength: 3, a sibling required list
dropped the referenced one, and sibling properties replaced the
referenced ones.

Annotations, which describe rather than constrain, still override the
referenced ones. Every other sibling keyword now goes into one more allOf
entry of the copy the loader makes, so Value keeps the shape of the
referenced schema. getkin#1271 reaches the same validation results by making
the siblings the Value and the referenced schema an allOf entry, which
leaves Value without the type or properties of the referenced schema
whenever a $ref has even just a description next to it.

The $refs within the siblings are now resolved before they are applied,
since applyRefSiblings also runs from the callback that ends a cycle.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
They were dropped. An OpenAPI 3.1 schema's sibling keywords, which
constrain the referenced schema, were lost that way, and so was every
other field next to a $ref: a document did not survive a round trip.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
The loader records the $refs it is in the middle of resolving, to close
cycles: a $ref met again is filled in with the value once it is resolved.
It recorded them by $ref alone, so the same $ref written for a path item
and for one of its responses handed the path item to the response, which
panicked with "interface conversion: interface {} is *openapi3.PathItem,
not *openapi3.Response". They are now recorded by kind of object and $ref.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
…t points to

A Path Item Object's $ref is no Reference Object: the fields written next
to it belong to the same path item, and every version of the specification
leaves the result undefined only "in case a Path Item Object field appears
both in the defined object and the referenced object".

Any such field made the loader skip the $ref, silently dropping all the
operations of the referenced path item, and letting a $ref to nothing go
unnoticed. Both are now merged; a field set on both sides is taken from
next to the $ref, as a Reference Object's summary and description are,
which is what OpenAPI 3.1 says this $ref is likely to be aligned with.

Marshaling writes back the $ref with the fields that were next to it,
instead of the $ref alone.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
…ma $ref

Running the test cases of getkin#1271 against the previous commits shows four
ways the keywords written next to an OpenAPI 3.1 schema's $ref still go
wrong:

* InternalizeRefs made a component of a $ref's Value, which is a copy of
  the referenced schema with the siblings applied, so they leaked into
  every other use of that component. It now uses the referenced schema,
  which SchemaRef keeps for this. And the external $refs within the
  siblings were rewritten in memory but marshaled as they were first read,
  so the internalized document still pointed outside. Marshaling now takes
  each sibling from what the parsed siblings now hold, and falls back to
  what was read for the fields a Schema leaves out, such as pattern: "".

* A file of bare schemas has no openapi field, so the siblings of the
  $refs within it were ignored as in OpenAPI 3.0, even when an OpenAPI 3.1
  document referred to it. Such a file now follows the version of the
  document whose refs the loader resolves.

* Siblings that do not form a schema, such as allOf: {type: integer},
  were silently dropped. OpenAPI 3.1 reads them as a Schema Object, so
  loading now fails there. OpenAPI 3.0 still ignores them.

* The allOf entry made of the assertion siblings had no Origin, so the
  errors it raised, such as for an invalid pattern, could not be located.
  It now shares the $ref's.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
These are the test cases of getkin#1271, which makes the keywords next to an
OpenAPI 3.1 schema's $ref the Value and the referenced schema an allOf
entry of it. Here Value keeps the shape of the referenced schema, so the
assertions on where each keyword ends up now check what the schema
accepts, or the shape it has here, instead:

* Fields next to a $ref round-trip as written, where getkin#1271 writes them
  out as an ordinary Schema would and so drops their zero values.
* In OpenAPI 3.0, where these fields are ignored, the loader does not
  follow the $refs within them, so InternalizeRefs leaves them as written.
  getkin#1271 internalizes them.
* A cyclic Go value is no longer validated: it overflows the stack
  against any recursive schema, siblings or not, and decoding JSON
  cannot produce one.

The four cases of $refs that only lead to one another are skipped. Their
Value is nil, with or without siblings, as on master: JSON Schema
2020-12 leaves such infinite recursion undefined, and getkin#1271 resolves them
to the conjunction of their siblings.

Adapted by Pierre Fenoll from getkin#1271

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
Found by seeding FuzzToV3 with the APIs-guru corpus.
…AML path

Seeds:
- Walk testdata trees rather than globbing their top level, so specs in
  origin/, issue*/, relativeDocs/ and the like seed the fuzzers too:
  FuzzLoadFromData goes from 51 seeds to 161.
- Add the APIs-guru corpus documents under 16 KiB (about 280 OpenAPI 3
  and 550 Swagger 2) when the TestV{2,3}ApisGuruOpenapiDirectory tests
  have downloaded it, outside -short. FuzzLoadFromData reaches 449.
- Add YAML-specific seeds: anchors and aliases, merge keys, tags, block
  scalars, YAML 1.1 implicit resolution, flow style and CRLF.

Targets:
- FuzzLoadWithOrigins: asking for origins changes nothing about the
  document, and every recorded position lies inside the input.
- FuzzYAMLMatchesJSON: a JSON document decodes the same through the
  YAML path, and any document survives being re-encoded as YAML.
- FuzzUnmarshalFromData: an openapi2 document loads back to itself from
  its own JSON and YAML forms.

internal/fuzzing holds what targets in several packages share: finding
seeds, and telling which documents the YAML emitter cannot write back.
The fuzz workflow runs the new targets and restores the corpus from the
go workflow's cache, fetching it when that is missing.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The loader refuses a $ref to another document when
IsExternalRefsAllowed is false, and reported it with fmt.Errorf, so the
message was the only thing telling that refusal apart from a document
that failed to load for some other reason. A caller that wants to name
the setting as the remedy, or map the refusal to its own error or exit
code, had to match the text.

Return *DisallowedExternalRefError instead, carrying the offending $ref.
The message is unchanged, so a consumer matching the text still works.

Both places that apply the setting go through allowsExternalRefs: a $ref
naming a whole document (loadSingleElementFromURI) and a $ref with a
fragment (resolveRefPath). The test covers each.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017e285CvofYf6joqC98gTXz
@reuvenharrison
reuvenharrison force-pushed the feat/typed-disallowed-external-ref branch from a74f267 to caa1af1 Compare October 26, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants