feat: swagger viewer - #90
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe Swagger viewer now normalizes and displays detailed OpenAPI endpoints, supports parameterized request execution and cURL copying, and uses a server-side proxy with DNS pinning, redirect validation, SSRF protection, response limits, and timeout handling. ChangesSwagger endpoint model and viewer
Try-it-out request flow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant SwaggerViewer
participant TryItOutPanel
participant TryItOutRoute
participant DNS
participant Upstream
Browser->>SwaggerViewer: select endpoint
SwaggerViewer->>TryItOutPanel: render endpoint inputs
TryItOutPanel->>TryItOutRoute: POST /api/try-it-out
TryItOutRoute->>DNS: resolve public destination
DNS-->>TryItOutRoute: pinned address
TryItOutRoute->>Upstream: send validated request
Upstream-->>TryItOutRoute: return response
TryItOutRoute-->>TryItOutPanel: return status, headers, and body
TryItOutPanel-->>Browser: render response details
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/try-it-out/route.ts`:
- Around line 265-278: Cap the response body accumulated by the chunks buffer in
the response handling flow, enforcing a maximum byte size while processing data
events. Reject or otherwise abort the response once the limit is exceeded, and
preserve the existing Buffer.concat resolution for responses within the limit.
In `@src/utils/swagger-editor/get-swagger-endpoints.ts`:
- Around line 3-45: Use the canonical SwaggerEndpoint, SwaggerEndpointParameter,
and SwaggerEndpointRequestBody types from this module throughout
endpoint-details.tsx, swagger-viewer.helpers.ts, try-it-out-panel.tsx, and
swagger-viewer.tsx. Remove the duplicate type literals in those files and import
the shared types instead, preserving existing usage and exports as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2722c27f-72d8-4110-851d-be85fa46b124
📒 Files selected for processing (18)
src/app/api/try-it-out/route.test.tssrc/app/api/try-it-out/route.tssrc/components/swagger-editor/swagger-editor.test.tsxsrc/components/swagger-editor/swagger-editor.tsxsrc/components/swagger-viewer/curl-command-preview.tsxsrc/components/swagger-viewer/endpoint-details.tsxsrc/components/swagger-viewer/swagger-viewer.helpers.tssrc/components/swagger-viewer/swagger-viewer.test.tsxsrc/components/swagger-viewer/swagger-viewer.tsxsrc/components/swagger-viewer/try-it-out-panel.test.tsxsrc/components/swagger-viewer/try-it-out-panel.tsxsrc/components/swagger-viewer/try-it-out-response-details.tsxsrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/utils/swagger-editor/get-method-color.tssrc/utils/swagger-editor/get-swagger-endpoints.test.tssrc/utils/swagger-editor/get-swagger-endpoints.tssrc/utils/swagger-editor/group-endpoints-by-path.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/api/try-it-out/route.ts (4)
124-129: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not replay credentials and bodies across redirects.
initremains unchanged for every redirect. A cross-origin 301/302/303 can therefore forward the originalAuthorization,Cookie, body, and method to the redirect destination. Restrict redirects to the same origin, or rebuild the request and strip sensitive headers/body when the origin or scheme changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.ts` around lines 124 - 129, Update the redirect-following loop around currentUrl and the unchanged init so credentials, request bodies, and methods are not replayed to a different origin or scheme. Either reject redirects that leave the original origin, or rebuild init for such redirects by stripping sensitive headers and the body and applying the appropriate redirect method; preserve same-origin redirect behavior.
355-358: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftApply the timeout to DNS resolution too.
The 15s timer starts only afterlookup()returns, so a slow resolver can keep the route pending past the advertised deadline. Redirects re-enterresolvePublicAddress(), so each hop repeats the uncapped DNS wait.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.ts` around lines 355 - 358, Update resolvePublicAddress’s DNS lookup path so the existing 15-second deadline also covers await lookup(hostname, { all: true, verbatim: true }). Ensure the timeout is applied per resolution attempt, including redirect hops, while preserving the existing address handling for literal IPs and successful DNS results.
61-83: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire auth before proxying.
saveRequestHistorySafelyonly skips persistence for anonymous callers, so unauthenticated users can still send arbitrary outbound requests throughfetchValidatedUrl. Add a session check before the fetch, and consider rate limiting/quotas too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.ts` around lines 61 - 83, Update POST to require an authenticated session before calling fetchValidatedUrl, using the existing session/authentication helper and returning the established unauthorized response when no session exists. Place this check after payload validation and before the outbound fetch; do not allow unauthenticated requests to reach proxying, and leave saveRequestHistorySafely behavior unchanged.
317-328: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject incomplete upstream closes. In
src/app/api/try-it-out/route.ts:317-328, a response that closes beforeendcan leave this promise pending whilerequestMessage.closealready clears the timeout. Reject non-endcloses and guard against double settlement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.ts` around lines 317 - 328, Update the response promise handling around the response error/data/end listeners to track whether it has settled, reject when the upstream response closes before the end event, and ignore subsequent events after settlement. Preserve the existing resolved response construction for normal end events and ensure requestMessage.close cannot leave the promise pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/try-it-out/route.ts`:
- Around line 86-94: Sanitize the endpoint before passing it to
saveRequestHistorySafely in both the success and error history-recording paths.
Update the shared URL handling around url.toString() to remove userinfo and
redact or omit query parameters containing credentials or secrets, then reuse
the sanitized endpoint for both records.
- Around line 86-94: Decouple saveRequestHistorySafely from the critical
response path in the route handler: ensure Supabase authentication and insertion
cannot delay the response by applying bounded cancellation/timeout, or
dispatching persistence through a non-critical background mechanism. Preserve
history recording when possible, and add explicit error handling for rejected
asynchronous persistence calls at the referenced history-save sites.
- Around line 379-388: Update the request_history persistence in the surrounding
try-it-out handler to capture the result of the Supabase insert and inspect its
error field. Log or otherwise handle the returned error without interrupting the
request flow, preserving history persistence as best-effort rather than relying
only on the surrounding catch.
---
Outside diff comments:
In `@src/app/api/try-it-out/route.ts`:
- Around line 124-129: Update the redirect-following loop around currentUrl and
the unchanged init so credentials, request bodies, and methods are not replayed
to a different origin or scheme. Either reject redirects that leave the original
origin, or rebuild init for such redirects by stripping sensitive headers and
the body and applying the appropriate redirect method; preserve same-origin
redirect behavior.
- Around line 355-358: Update resolvePublicAddress’s DNS lookup path so the
existing 15-second deadline also covers await lookup(hostname, { all: true,
verbatim: true }). Ensure the timeout is applied per resolution attempt,
including redirect hops, while preserving the existing address handling for
literal IPs and successful DNS results.
- Around line 61-83: Update POST to require an authenticated session before
calling fetchValidatedUrl, using the existing session/authentication helper and
returning the established unauthorized response when no session exists. Place
this check after payload validation and before the outbound fetch; do not allow
unauthenticated requests to reach proxying, and leave saveRequestHistorySafely
behavior unchanged.
- Around line 317-328: Update the response promise handling around the response
error/data/end listeners to track whether it has settled, reject when the
upstream response closes before the end event, and ignore subsequent events
after settlement. Preserve the existing resolved response construction for
normal end events and ensure requestMessage.close cannot leave the promise
pending.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f1d66f4a-f778-47ce-8988-e3a056276ed7
📒 Files selected for processing (2)
src/app/api/try-it-out/route.test.tssrc/app/api/try-it-out/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/try-it-out/route.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/app/api/try-it-out/route.test.ts (4)
465-486: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing history-insertion assertion for DNS timeout.
Similar to the other new edge-case tests, this timeout test doesn't verify that
mocks.insertwas called with thetimeouterror details, unlike how earlier tests validate history recording for their respective error paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.test.ts` around lines 465 - 486, Extend the DNS-resolution timeout test around POST to assert that mocks.insert was called with the timeout error details, matching the history-recording assertions used by the other error-path tests. Keep the existing response status, error payload, and httpsRequestMock assertions unchanged.
417-441: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing history-insertion assertion for the blocked cross-origin redirect.
The analogous private-destination blocking tests (Lines 205-247) assert
mocks.insertrecords theblockedUrlerror/details. This redirect-block test verifies request/response behavior but not that the failure was recorded to history, leaving that side effect unverified for this distinct code path (mid-flow redirect rejection vs. upfront validation).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.test.ts` around lines 417 - 441, Add a history-insertion assertion to the cross-origin redirect test around the POST call, verifying mocks.insert records the blockedUrl failure and corresponding error details, matching the analogous private-destination blocking tests while preserving the existing request and response assertions.
317-349: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing history-insertion assertion for the incomplete-close failure path.
Sibling
requestFailedtests (e.g., the request/response stream error cases) assert that the failure is recorded viamocks.insert. This test only checks the HTTP response, leaving the audit/history side effect for this specific incomplete-close edge case unverified.Suggested addition
expect(response.status).toBe(502); await expect(response.json()).resolves.toEqual({ errorCode: 'requestFailed' }); + expect(mocks.insert).toHaveBeenCalledWith( + expect.objectContaining({ status_code: null }), + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.test.ts` around lines 317 - 349, Extend the incomplete upstream close test in “rejects incomplete upstream closes before the end event” to assert that the requestFailed failure is recorded through mocks.insert, matching the sibling request/response stream error tests. Verify the inserted history entry for this specific close-before-end path while preserving the existing 502 response assertions.
164-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated request-construction boilerplate across the suite.
The
new Request('http://localhost/api/try-it-out', { body: JSON.stringify(...), method: 'POST' })pattern is duplicated across most tests in this ~630-line file. Extracting a smallcreateTryItOutRequest(payload)helper would reduce duplication and improve readability without changing behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/try-it-out/route.test.ts` around lines 164 - 172, Extract the repeated POST Request construction in the try-it-out test suite into a createTryItOutRequest(payload) helper, then update the affected POST calls to use it while preserving each payload and existing test behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/api/try-it-out/route.test.ts`:
- Around line 465-486: Extend the DNS-resolution timeout test around POST to
assert that mocks.insert was called with the timeout error details, matching the
history-recording assertions used by the other error-path tests. Keep the
existing response status, error payload, and httpsRequestMock assertions
unchanged.
- Around line 417-441: Add a history-insertion assertion to the cross-origin
redirect test around the POST call, verifying mocks.insert records the
blockedUrl failure and corresponding error details, matching the analogous
private-destination blocking tests while preserving the existing request and
response assertions.
- Around line 317-349: Extend the incomplete upstream close test in “rejects
incomplete upstream closes before the end event” to assert that the
requestFailed failure is recorded through mocks.insert, matching the sibling
request/response stream error tests. Verify the inserted history entry for this
specific close-before-end path while preserving the existing 502 response
assertions.
- Around line 164-172: Extract the repeated POST Request construction in the
try-it-out test suite into a createTryItOutRequest(payload) helper, then update
the affected POST calls to use it while preserving each payload and existing
test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1813009b-3db1-48e0-bb63-ff357860138c
📒 Files selected for processing (2)
src/app/api/try-it-out/route.test.tssrc/app/api/try-it-out/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/try-it-out/route.ts
3 participants
Summary
Completes Feature 4 Swagger Viewer work in one feature PR.
Closes #32
Closes #45
Closes #64
Closes #65
Closes #66
Closes #67
Closes #76
Closes #77
What Changed
schema.info./api/try-it-out.endpoint-details.tsxtry-it-out-panel.tsxcurl-command-preview.tsxtry-it-out-response-details.tsxswagger-viewer.helpers.tsAffected Files
src/app/api/try-it-out/route.tssrc/app/api/try-it-out/route.test.tssrc/components/swagger-editor/swagger-editor.tsxsrc/components/swagger-editor/swagger-editor.test.tsxsrc/components/swagger-viewer/swagger-viewer.tsxsrc/components/swagger-viewer/swagger-viewer.test.tsxsrc/components/swagger-viewer/endpoint-details.tsxsrc/components/swagger-viewer/try-it-out-panel.tsxsrc/components/swagger-viewer/try-it-out-panel.test.tsxsrc/components/swagger-viewer/curl-command-preview.tsxsrc/components/swagger-viewer/try-it-out-response-details.tsxsrc/components/swagger-viewer/swagger-viewer.helpers.tssrc/utils/swagger-editor/get-swagger-endpoints.tssrc/utils/swagger-editor/get-swagger-endpoints.test.tssrc/utils/swagger-editor/get-method-color.tssrc/utils/swagger-editor/group-endpoints-by-path.tssrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonValidation
npm testnpm run lintnpm run buildgit diff --checkmerge-tree origin/develop HEADNotes
This PR intentionally does not include request History persistence. Try-It-Out currently executes the request and displays response data in the
Viewer. Saving executed requests to History belongs to Feature 5 integration and should be wired to the History service/table separately.
Summary by CodeRabbit