fix(sat): add missing 4xx/5xx error responses across the SAT API#37
Open
dmchaledev wants to merge 1 commit into
Open
fix(sat): add missing 4xx/5xx error responses across the SAT API#37dmchaledev wants to merge 1 commit into
dmchaledev wants to merge 1 commit into
Conversation
Closes #29. sat/openapi.yaml documented happy-path responses only on most of its ~40 operations, leaving codegen tools and integrators with no idea what error shapes to expect. - Add shared components/responses (Unauthorized, NotFound, BadRequest, InternalError) backed by the existing Response schema. - Add 401 + 500 to every authenticated operation. - Add 404 to every single-resource GET/PUT/DELETE keyed by {id}. - Add 400 to every POST/PUT that accepts a JSON request body. - Give the existing bare 403 entries (already documented as permission-gated in their descriptions) proper response content instead of a description-only stub. 403 was intentionally left off endpoints whose descriptions don't already state a permission requirement, to avoid asserting RBAC behavior the spec doesn't document elsewhere. Verified with the same commands CI runs: `spectral lint sat/openapi.yaml --ruleset .spectral.yaml --fail-severity error` -> 0 errors (only the pre-existing, intentionally-advisory trailing-slash warnings). Also cross-checked with `redocly lint` to confirm no dangling $refs.
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
2 participants
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.
Summary
Closes #29.
sat/openapi.yamldocumented happy-path responses on nearly all of its ~40 operations but omitted standard error responses almost everywhere, so codegen tools (openapi-generator, oapi-codegen) and integrators reading the spec have no idea what error shapes to expect.Note on process: this repo currently has an open duplicate-PR problem (#34) — 27+ open PRs, many overlapping. Before starting, I checked all open issues/PRs touching
sat/openapi.yaml,asm/openapi.yaml, anddocs/index.html. Issue #29 is real and unimplemented, and no open PR (including the SRI-hash PRs #19/#36 and the CONTRIBUTING.md PR #35) touches this. This PR only changessat/openapi.yaml.Changes
components/responses(Unauthorized,NotFound,BadRequest,InternalError) backed by the existingResponseschema, to avoid repeating the same error shape 100+ times.401+500to every authenticated operation (all of them except/health,/ready, and the unauthenticated/sso/saml/*browser-facing endpoints, which already had appropriate responses for their flow).404to every single-resourceGET/PUT/DELETEkeyed by{id}that was missing it (most of Groups/Templates/Pages/Sending Profiles/Webhooks had none at all).400to everyPOST/PUTthat accepts a JSON request body and didn't already document one.403entries (already documented as permission-gated in their own descriptions —createCampaign,getCampaignResults,approveCampaign,rejectCampaign,listRoles,listUsers,createUser) proper response content instead of a description-only stub.Deliberately not done: I did not add
403to endpoints whose descriptions don't already state a specific permission requirement (e.g. Groups/Templates/Pages/SMTP CRUD). The spec doesn't document per-endpoint RBAC there, and I didn't want to assert behavior I can't verify. Happy to extend this once that's confirmed against the actual SAT permission model — flagging it here rather than guessing.Test plan
spectral lint sat/openapi.yaml --ruleset .spectral.yaml --fail-severity error(same command CI runs) → 0 errors, only the 8 pre-existing trailing-slash warnings that.spectral.yamlalready documents as intentional (SAT genuinely uses trailing-slash paths).redocly lint sat/openapi.yamlas an independent cross-check → no dangling$refs introduced.asm/openapi.yamluntouched, re-linted to confirm it's still clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01GnwDV2BgGLXeqoNVP6qKnA
Generated by Claude Code