Skip to content

fix(compilers/openapi): report an oauth2 scheme with no flow - #727

Open
fuad-daoud wants to merge 2 commits into
mainfrom
fix/oauth2-no-flows
Open

fuad-daoud wants to merge 2 commits into
mainfrom
fix/oauth2-no-flows

Conversation

@fuad-daoud

Copy link
Copy Markdown
Collaborator

Summary

{type: oauth2, flows: {}} is structurally valid — every member of OAuthFlows is optional — so it lowers to an AuthKindOAuth2 scheme with an empty flow list and no report from anywhere. An oauth2 scheme states no token endpoint without a flow, so the entry is unusable, and a requirement naming it keeps a live AuthID for a scheme that says nothing an emitter can implement (#646).

Rule. An entry that lowers to AuthKindOAuth2 whose lowered flow list is empty is reported once, at the entry's own components/securitySchemes/<name> pointer. One rule for every spelling: flows absent, flows: {}, or a flows object holding only keys this model does not name.

Code, severity, placement. New code openapi/oauth2-no-flow (diag.OAuth2NoFlow), warning, at the entry. The placement is the half the load phase cannot supply: flows: {} is structurally valid, so the loader has nothing to refuse there, and the spelling it does refuse, flows absent, it reports as validation-required-field carrying no pointer at all, which sites an unreferenced entry nowhere.

The scheme still interns — deliberately not the refusal #294 points at. #294 refused an entry naming no mechanism because ir.AuthKind has no value for "the document did not say which mechanism": interning would have asserted that the API is authenticated by nothing in particular. Here the IR states exactly what the document said — oauth2, with no flows — so nothing is misstated and nothing must be refused. Refusing would also delete declared text (description, x-*, oauth2MetadataUrl) and collapse every requirement naming the scheme, for a document defect the IR can hold. The compiler already interns the two equally unusable siblings — apiKey with no name, openIdConnect with no URL — so wire-level usability is not the line it draws; the mechanism token is (fillSchemeKind), and that line is unchanged.

Warning, not error. The document lowers whole, and the shape is the same class as DisjointVisibility (exactly represented, unusable), EmptyEnum, ReservedHeaderName, InvalidMethodKey and InvalidLocationKeyword. An error would also stop harness.Check at the first error diagnostic and keep any fixture carrying the shape out of irverify; --fail-on warning is the caller's lever.

The metadata URL does not exempt the entry. The finding is that the flows object declares no flow, true whatever else the entry says, and the loader's requiredness rule is dialect-independent, firing for the absent spelling on 3.2 too. The URL is preserved on the scheme either way.

Where it lives.

  • compilers/openapi/internal/diag/diag.go: diag.OAuth2NoFlow with its design record, immediately after IncompleteSecurityScheme.
  • compilers/openapi/internal/auth/auth.go: oauthNoFlowDiag beside mechanismRefusalDiag, and the branch after the named guard in lowerSecurityScheme. fillSchemeKind's oauth2 case records that an absent or empty flows object lowers to nil flows, and LowerSecuritySchemes' doc now names both reporters of an entry that did intern.
  • No ir/ type, no Options switch, no irverify rule: nothing is inferred, and holding flow fields in irverify would re-validate the source.

Test plan

  • Focused: go test ./compilers/openapi/internal/auth/ -run 'TestLowerSecuritySchemes|TestAuth_|TestSecurityRequirement' green.
  • TestLowerSecuritySchemes_AnOAuth2SchemeWithNoFlowIsReported pins the rule by spelling: flows: {} (the issue repro — one warning at /components/securitySchemes/s, severity warning, message names the scheme), flows absent (the same warning beside the loader's unpointed finding), flows: {application: …} (the new warning plus the existing unknown-object-key at /components/securitySchemes/s/flows/application), flows: {} with oauth2MetadataUrl (the warning still fires and the URL is kept), one declared flow and one declared device flow (the false arm — no diagnostic with the code), and an entry with no type (only incomplete-security-scheme). Every interned row also pins that the requirement naming the scheme keeps a live AuthID.
  • TestLowerSecuritySchemes_AFlowlessRefEntryIsReportedAtEachEntry: a $ref alias to a flowless declaration reports at both /components/securitySchemes/alias and /components/securitySchemes/target, each naming its own entry — openapi: referenced non-schema components hoist once per use site under fabricated pointers #107's "an alias and its target are two named schemes".
  • TestAuth_OAuthNoFlowsUnknownTypeAndGhostRef is extended with the placed-warning assertions; it was the seed's "pins today's silence" case.
  • Mutations, each planted, observed and reverted:
    • delete the branch → TestAuth_OAuthNoFlowsUnknownTypeAndGhostRef, TestLowerSecuritySchemes_AFlowlessRefEntryIsReportedAtEachEntry and all four reporting rows of the table test redden;
    • fire only for absent flows (ss.GetFlows() == nil) → the flows: {}, {application: …} and metadata rows redden, plus TestLowerSecuritySchemes_ARefdEntryRecordsFieldsWhereTheyAreWritten;
    • report at the flows pointer instead of entry → the pointer assertions redden;
    • flip the severity to error → the severity assertions redden;
    • run the check before the named guard with no Kind test → the declared-flow rows redden, and the untyped-entry row too once the refusal's early return is made to carry the accumulator, since until then it discards what was appended.
  • Coverage: scripts/check-coverage.sh → Coverage gate passed: all 7599 statements covered. Both arms of the new branch are exercised: the declared-flow rows reach it false, the rest true.
  • make gate (fmt, vet, lint, nolint-grammar, nolint, build, coverage-count, coverage, fuzz, bench-smoke) passes with golangci-lint reporting 0 issues.
  • Fixtures checked and left alone. Every spec under testdata/ that declares type: oauth2 declares a flow — golden/openapi/petstore.yaml (implicit), conformance/openapi/security-schemes.yaml (clientCredentials), conformance/openapi/extensions-x.yaml (implicit), openapi/unknown_keys.yaml (implicit) — so no golden, diagnostic list, conformance assertion or fixture changes, and no testdata/ file is touched by this PR.

Deliberately out of scope

  • A flow that is present but incomplete (missing tokenUrl, authorizationUrl or scopes): that is the loader's own finding, and not what "declares no flow" means.
  • An irverify rule: holding flow fields there would re-validate the source, which its own doc rules out.
  • The library's unpointed securityScheme.flows is required finding is left exactly as it is. This change adds a placed report beside it rather than replacing or suppressing it.

Closes #646.

@fuad-daoud fuad-daoud self-assigned this Sep 30, 2026
@fuad-daoud fuad-daoud changed the title fix(compilers/openapi): report an oauth2 scheme with no flow (#646) fix(compilers/openapi): report an oauth2 scheme with no flow Sep 30, 2026
Resolve the conflicts with main and bring the doc comments this branch
adds under the 100-word cap main now enforces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GCzyGPq9DQ7RLfagZm2JB5
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.

openapi: an OAuth2 scheme with no flows compiles clean

1 participant