fix(compilers/openapi): refuse a scheme that names no mechanism - #296
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix(compilers/openapi): refuse a scheme that names no mechanism#296OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
A securitySchemes entry with no `type` fell into fillSchemeKind's default branch, where the unrecognised-type degradation interned it as a custom scheme whose mechanism was the empty string. A requirement naming it got a live AuthID, so the IR stated that the API was authenticated by a scheme that named nothing. `type: http` with no `scheme` token reached the same state through fillHTTPScheme's default branch. Both are now refused: nothing is interned, the entry is reported at its own components pointer under a new openapi/incomplete-security-scheme code, and a requirement naming it drops whole as one naming an undeclared scheme already does. AuthKind has no value meaning "the document did not say" — AuthKindCustom names a mechanism the IR does not model, not one the entry never gave — and inventing one would put a scheme no emitter can implement in Document.Auth, recognisable only by an empty Scheme field. The same branches also dropped whatever else the entry declared. Each mechanism's lowering reads only the fields its own type defines — `in` and `name` under apiKey, `flows` under oauth2 — so a document writing them elsewhere lost them with no field, no Unmodeled entry and no diagnostic. Every field a type does not define is now kept verbatim under Unmodeled with ReasonDegradedLowering, located at the field itself and announced. Filling the IR field of the same name instead would say the mechanism has a property it does not define. The list of per-type fields is held to the upstream model by reflection, so a field that model gains fails a test rather than silently vanishing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #294.
Summary
A
components.securitySchemesentry with notypefell intofillSchemeKind's defaultbranch — the one written for a
typethe compiler does not recognise, which degrades toAuthKindCustomcarrying the raw token. With notypeat all there is no token, so theentry interned as a custom scheme whose mechanism was the empty string, and a requirement
naming it got a live
AuthID. The IR stated that the API was authenticated by a schemethat named nothing.
type: httpwith noschemereaches the same state throughfillHTTPScheme's defaultbranch. It is the same defect at a second site and is fixed with it.
Both are now refused. Nothing is interned, the entry is reported at its own components
pointer under a new
openapi/incomplete-security-schemecode, and a requirement naming itdrops whole exactly as one naming an undeclared scheme already does.
Why refuse rather than intern with a distinguishable spelling.
AuthKindhas no valuemeaning "the document did not say" — every kind names a mechanism, and
AuthKindCustomnames one the IR does not model rather than one the entry never gave. Interning it would
put a scheme no emitter can implement in
Document.Auth, recognisable only by an emptySchemefield: an in-band error for every consumer to rediscover. Adding an enum memberfor it would put a description of a broken document in the channel that describes APIs,
when the diagnostics channel already exists for that.
This reuses the remedy an option naming an undeclared scheme already gets, and does so
deliberately rather than by inheritance. The two faults differ — that entry was never
declared, this one was declared and left unsaid — but the IR has no more room for the
second than for the first, and the cost is one already documented and diagnosed at every
step: a requirement naming the entry drops whole, and a list whose every option drops
collapses to
nilrather than to the empty-option encoding that reads as "no auth is alsofine". The reasoning is recorded at
mechanismRefusalDiag.The declared fields no longer vanish
The same branches dropped whatever else the entry wrote, and so did every other branch.
Each mechanism's lowering reads only the fields its own type defines —
in/nameunderapiKey,flowsunderoauth2, and so on — so a document writing them anywhere elselost them outright: no IR field, no
Unmodeledentry, no diagnostic. That is a silent dropof declared source text.
Every mechanism field a type does not define is now kept verbatim under
Unmodeledatopenapi:<field>, located at the field's own pointer, markedReasonDegradedLowering, andannounced with an info diagnostic.
ir.AuthSchemeis flat and does hold a field of eachname, but filling one would say the mechanism has a property it does not define — an apiKey
location on a scheme that is not an apiKey — so the declaration is kept beside the scheme
rather than inside it.
The one collision is deliberate: an unrecognised type already spends
AuthScheme.Schemeonthe type token, so a
schemewritten beside it is kept raw even though the same fieldwould have held it under
type: http. Which one wins is unchanged.The per-type field list is held to the upstream
SecuritySchememodel by reflection, so afield that model gains fails a test rather than silently vanishing — the shape of this bug,
reintroduced by an upstream bump.
Deliberately out of scope
irverifyhas noAuthSchemerules at all, so nothing structurally asserts that aninterned scheme names a mechanism. The compiler-side refusal closes the instance; the class
is filed as #295, which also notes why such a rule would not have caught this one through
the harness (every document that reaches this state carries a loader error, and
harness.Checkreturns at the first one).Test plan
gofmt,go vet,golangci-lint run(0 issues),go build ./..., and./scripts/check-coverage.shall clean — 100% of 4942 statements.preserved fields across all six type classes (each declaring all seven mechanism fields,
so every row asserts both halves of the one-home rule), for the extension and preserved
fields sharing one map, and for a field whose value JSON cannot hold being reported
rather than announced as kept.
is what puts the lossless-capture claim in the corpus; the golden records it.
interning again, the http entry interning again, the preservation removed, the wrong
Unmodeledreason, an unrecognised type keeping nothing, the refused entry internedanyway, extensions clobbering the preserved fields,
apiKeywrongly claimingbearerFormat, the corpus dropping the stray field, a mislocated entry, the refusaldowngraded to a warning, and the announcement removed. All twelve were killed, each by
the test named for it and by an assertion rather than a compile error. The reflection
guard was checked in both directions.