Fix #802: report apiKey security scheme name changes with old and new values

Draft
IN
20h ago
+1123-8

Fixes #802

Reworked after the review on this PR (#939 (review) — DrSatyr, CHANGES_REQUESTED). The scheme matching was rebuilt around the reviewer's four-case spec and the renderers were made consistent.

Behavior per the review spec

An apiKey scheme has a component key (PAT) and a parameter name (the header/query/cookie clients send). Actual results on this branch, verified with the CLI on valid fixtures (operation-level security, PAT: []):

#ChangeResultVerdict
1key renamed, name sameno change reportedno_changes
2same key, name changedChanged security scheme name: Authorization -> pat (the #802 case)incompatible via incompatible.security.scheme.apikey.name.changed (default true, can be switched off)
3both key and name changedremoved + added scheme (New security scheme / Deleted security scheme), no rename guessedincompatible
4new alternative requirement addedNew security requirement: petstore_auth: [read], nothing elsecompatible

What changed since the last push

  • Matching is now by component key first, then by parameter name (SecurityRequirementDiff). The previous name-first order mispaired schemes when a requirement contains several apiKeys — e.g. a parameter-name swap between two schemes was silently swallowed. Case 3 (both changed) falls through both matchers and is reported as removed + added; no rename is ever guessed.
  • Dropped the block that diffed schemes of newly added requirements (SecurityRequirementsDiff), as requested. It reported case 4 as breaking (and mis-attributed type changes). The ChangedSecurityRequirements#changedSchemes list was removed with it, which also eliminates the duplicate/misattributed-scheme-change risk in that loop.
  • SecuritySchemeDiff only compares the parameter name while the right-hand scheme is still an apiKey, so a same-key type change can no longer be reported as (and suppressed by) the apiKey-name rule.
  • Renderers are now consistent across console, markdown and html:
    • all three show schemes added to or removed from a changed requirement (console and html previously dropped them);
    • the html Changed security scheme : item is skipped when it has no displayable change, and description changes are now rendered there;
    • markdown security details use a two-space nested-list indent (* Changed security requirement: with * ... children) so they render as a list — the previous four-space indent rendered as an indented code block.

Review notes

  • Markdown still duplicates #933 for now: not rebased yet because #933 is unmerged (rebasing now would break this branch). Once #933 lands I will rebase and build on its code. The same applies to the related #934/#937/#938.
  • cubic-dev-ai[bot] issues (9): fixed in this push — the malformed fixtures (issue-802-api-key-1.yaml, issue-802-api-key-2.yaml → valid operation-level security, empty PAT: []), the SecurityRequirementDiff fallback ordering (superseded by the key-first matching above), the SecuritySchemeDiff changedName guard (only apiKey→apiKey), the MarkdownRender four-space indent (now a proper nested list), the SecurityRequirementsDiff duplicate/misattributed-scheme loop (block removed entirely), the empty html Changed security scheme item (suppressed, description now rendered), and the console missing New security scheme / Deleted security scheme lines (now rendered). Refuted: the MarkdownRender extension-only OAuth flow issue (extension-only flow changes vanish from markdown) — the model only flags a flow as changed when a URL changes (ChangedOAuthFlow#isCoreChanged returns no_changes otherwise), and none of the renderers render extension diffs for security entities; rendering extension diffs is a separate feature, not something this PR introduced. Happy to take it in a follow-up.

Verification

./mvnw test green: core 285 tests, cli 13 (baseline was core 277 + cli 13; +8 from the per-case tests). New per-case tests: SecurityDiffTest (case 1–4 plus a two-apiKey name-swap regression proving key-first pairing), render tests per renderer for the name change and the removed+added shape, SecuritySchemeBCTest#apiKeyNameChanged for the on/off switch.

CLI output on the fixtures (console renderer):

# case 2 (#802)
- GET    /info
  Security Requirements:
    - Changed security requirement: PAT
      * Changed security scheme name: Authorization -> pat
                 API changes broke backward compatibility

# case 3
    - Changed security requirement: PAT2
      * New security scheme: PAT2: []
      * Deleted security scheme: PAT: []

# case 4
    - New security requirement: petstore_auth: [read]
                 API changes are backward compatible

Markdown (case 2) now renders as a proper nested list:

###### Security Requirements:

* Changed security requirement: `PAT`
  * Changed security scheme name: `Authorization` -> `pat`

Original description below.


What

A changed name under components.securitySchemes for an apiKey scheme is now captured in the diff model and rendered with old and new values in all three renderers:

  • console: * Changed security scheme name: Authorization -> pat
  • markdown: * Changed security scheme name: Authorization->pat``
  • html (--html-detailed): Changed security scheme : name: Authorization -> pat

It also feeds the backward-compatibility verdict through a new option, incompatible.security.scheme.apikey.name.changed (default true), so the check can be disabled the same way as the other security-scheme rules — the reporter specifically noted there was no switch for it. Changing the parameter name means existing clients keep sending the old header/query/cookie name, so incompatible-by-default seemed like the honest default.

Why

The model knew a security scheme changed but never recorded which field, so the renderers had nothing line-level to print. One note on the repro specs attached to the issue: in both old.json and new.json the securitySchemes block sits under paths (not components), and the security block sits at path-item level. Both are silently dropped by the swagger parser, so as attached, no diff engine can see the scheme definitions at all — which is why only a bare changed operation shows up. The fixtures in this PR use valid locations.

How

  • SecuritySchemeDiff sets changedName for APIKEY schemes; ChangedSecurityScheme#isCoreChanged gates the incompatibility on the new option (same handling as the existing changedScheme flag).
  • Console, markdown and html read the old and new values from the SecurityScheme objects the model already carries.

Summary by cubic

Captures apiKey security scheme name changes so console, markdown, and HTML renderers report the old and new values instead of a bare operation-level change, and aligns the security renderers.

  • A new incompatible.security.scheme.apikey.name.changed option (default true) controls the backward-compatibility verdict, since existing clients keep sending the old parameter name.
  • SecurityRequirementDiff pairs requirement schemes by component key first and only falls back to the parameter name when the key no longer matches, so a rename is diffed as a scheme change instead of a deleted plus re-added requirement, and several apiKey schemes in one requirement are no longer mispaired by name.
  • Schemes referenced by newly added security requirements are no longer diffed against the other spec; a new alternative security requirement is compatible and reports no scheme changes.
  • Console, markdown, and HTML renderers list schemes added to or removed from a changed requirement; HTML skips scheme items with no displayable change and shows description changes; markdown uses a two-space nested-list indent so security details render as a list rather than a code block.