Fix #802: report apiKey security scheme name changes with old and new values
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: []):
| # | Change | Result | Verdict |
|---|---|---|---|
| 1 | key renamed, name same | no change reported | no_changes |
| 2 | same key, name changed | Changed security scheme name: Authorization -> pat (the #802 case) | incompatible via incompatible.security.scheme.apikey.name.changed (default true, can be switched off) |
| 3 | both key and name changed | removed + added scheme (New security scheme / Deleted security scheme), no rename guessed | incompatible |
| 4 | new alternative requirement added | New security requirement: petstore_auth: [read], nothing else | compatible |
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). TheChangedSecurityRequirements#changedSchemeslist was removed with it, which also eliminates the duplicate/misattributed-scheme-change risk in that loop. SecuritySchemeDiffonly 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-levelsecurity, emptyPAT: []), theSecurityRequirementDifffallback ordering (superseded by the key-first matching above), theSecuritySchemeDiffchangedNameguard (only apiKey→apiKey), theMarkdownRenderfour-space indent (now a proper nested list), theSecurityRequirementsDiffduplicate/misattributed-scheme loop (block removed entirely), the empty htmlChanged security schemeitem (suppressed, description now rendered), and the console missingNew security scheme/Deleted security schemelines (now rendered). Refuted: theMarkdownRenderextension-only OAuth flow issue (extension-only flow changes vanish from markdown) — the model only flags a flow as changed when a URL changes (ChangedOAuthFlow#isCoreChangedreturnsno_changesotherwise), 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
SecuritySchemeDiffsetschangedNameforAPIKEYschemes;ChangedSecurityScheme#isCoreChangedgates the incompatibility on the new option (same handling as the existingchangedSchemeflag).- Console, markdown and html read the old and new values from the
SecuritySchemeobjects 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.changedoption (defaulttrue) controls the backward-compatibility verdict, since existing clients keep sending the old parameter name. SecurityRequirementDiffpairs 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 severalapiKeyschemes 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.