Add OpenAPI 3.1 webhook support and per-operation security - #38
Draft
andreashasse wants to merge 6 commits into
Draft
andreashasse wants to merge 6 commits into
andreashasse wants to merge 6 commits into
Conversation
Wraps spectra's new webhook builders. - Spectral.OpenAPI.webhook/2,3 creates a webhook definition, keyed by an event name rather than a URL path, because the consumer owns the URL the API calls. - Spectral.OpenAPI.to_openapi/4 generates a spec from both endpoints and webhooks, emitting the latter under the document's top-level webhooks key. endpoints_to_openapi/2,3 are unchanged and emit no webhooks key. The direction is inverted relative to an endpoint: the request body is the payload the API sends out, and the responses describe what the consumer is expected to send back. add_response/2, with_request_body/3,4 and with_parameter/3 already pass their argument straight through, so webhooks build through the same pipe as endpoints with no wrapper changes, and webhook schemas share components/schemas with endpoints. :path and :query parameters raise on a webhook - the consumer owns the URL, so there is nothing to template into. mix.exs temporarily points spectra at the webhook branch; this must be reverted to a hex version before merge.
add_response/2, with_request_body/3,4 and with_parameter/3 pass their argument straight through to spectra, which accepts a webhook_spec() as well as an endpoint_spec(), but the Elixir @specs still declared only endpoint_spec(). Building a webhook through those functions was therefore a dialyzer contract violation for callers. Caught by phoenix_spectral's dialyzer, which does build webhooks that way. Spectral's own dialyzer run was clean because nothing in this repo passes a webhook to them.
spectra now accepts a security key in the Doc map for endpoint/3 and webhook/3, emitted on that operation alone and overriding the global security default; [] opts the operation out entirely. This is what lets an API authenticate inbound calls one way and sign its outgoing webhooks another. No wrapper code was needed - endpoint/3 and webhook/3 already pass the doc map straight through, and their @specs reference :spectra_openapi.endpoint_doc(), which gained the key upstream. The tests here are characterization tests: they prove the key actually survives the pass-through rather than being filtered, which is not obvious and would otherwise regress silently. They earned their keep immediately: they failed at first because mix.lock still pinned the previous spectra commit, so the suite was exercising the old dependency. Lock advanced to aabf317.
spectra's webhook branch merged main (0.14.0, which changed how doc annotations survive schema inlining). The lock still pointed at the pre-merge commit, so the suite was exercising spectra as it was before that change rather than what this PR will actually ship against. Suite is unchanged and green on the new lock: 21 doctests, 3 properties, 224 tests, plus credo --strict, dialyzer (0 errors), format --check and a warning-free forced compile. Still a branch dep - reverts to a hex version before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Erhgwu59rBxiYw1fVQGbcr
…k-support-sz755w # Conflicts: # CHANGELOG.md # mix.exs # mix.lock
spectra's webhook branch merged main again (0.14.1's struct-encode fix plus the OTP/rebar3/ELP pin bump) and moved to 1931e32. The lock still pointed at the pre-merge commit. Green on the new lock: 21 doctests, 3 properties, 252 tests, plus credo --strict, ex_dna, dialyzer (0 errors), format --check and a warning-free forced compile. Still a branch dep - reverts to a hex version before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Erhgwu59rBxiYw1fVQGbcr
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.
Elixir wrapper for the webhook builders and per-operation security added in andreashasse/spectra#188.
mix.exstemporarily points:spectraat theclaude/openapi-webhook-support-sz755wbranch (lock at1931e32). This must be reverted to a hex version before merge, once andreashasse/spectra#188 lands and is released —mainis on{:spectra, "~> 0.14.0"}, so this becomes whatever version ships the webhook support.API
A webhook is keyed by an event name rather than a URL path, because the consumer owns the URL the API calls. The direction is inverted relative to an endpoint: the request body is the payload the API sends out, and the responses describe what the consumer is expected to return.
endpoints_to_openapi/2,3are unchanged and emit nowebhookskey (covered by a test).Per-operation security
The doc map for both
endpoint/3andwebhook/3now takes:security, overriding the global default for that operation;[]opts it out entirely:Why the wrapper is this small
add_response/2,with_request_body/3,4andwith_parameter/3already pass their first argument straight through to Erlang, and spectra widened those to acceptwebhook_spec()as well asendpoint_spec(). Likewiseendpoint/3andwebhook/3pass the doc map through untouched, so:securityneeded no wrapper code at all — only the two new functions were genuinely new.The security tests are therefore characterization tests: they prove the key survives the pass-through rather than being filtered, which isn't obvious from the code and would otherwise regress silently. They earned their keep immediately — they failed on first run because
mix.lockstill pinned the previous spectra commit, so the suite was quietly exercising the old dependency.Tests
Twelve new tests: webhook construction with and without docs, building through the shared modifiers,
:path/:queryrejection, top-level emission, schema sharing between an endpoint and a webhook, absence of the key without webhooks, the:pre_encodedoption, a webhook overriding the global security requirement,[]opting out, and an endpoint overriding it too. Plus a doctest onwebhook/3.Verified locally on the merge head:
mix test(21 doctests, 3 properties, 252 tests),mix credo --strict,mix ex_dna,mix dialyzer(0 errors) andmix format --check-formattedall clean, andmix compile --force --warnings-as-errorsis warning-free.Kept current with
mainmainhas moved twice while this sat open — 0.14.0 (spectra's nested doc annotations) and then a batch of fixes including the codecsp_type_or_ref()gap, the unexpected-crash reraise, and the[elem, ...]shorthand.origin/mainis merged in here, and the:spectralock tracks the webhook branch's own merge head.The merge conflicts were confined to the dependency pins and the changelog:
mix.exsandmix.lockresolve to this PR's branch dep rather thanmain's hex version (spectra 0.14.1 has no webhook support yet), andCHANGELOG.mdkeeps both this PR's## [Unreleased]entries and the released sections below them. Everything else auto-merged.A bug this caught upstream
phoenix_spectral's dialyzer found that
add_response/2,with_request_body/3,4andwith_parameter/3still declaredendpoint_spec()only, even though the underlying Erlang functions had been widened to accept webhooks. Building a webhook through them was a contract violation for any caller. Fixed in 56e4091 — this repo's own dialyzer run was clean because nothing here passes a webhook to those functions.