Fix replay issues - #14
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 21 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds shared GraphQL parsing utilities, refactors replay creation to use target URLs and direct replay SDK calls, rebuilds GraphQL view-mode parsing and editing, updates view-mode registration, and pins config and package versions. ChangesGraphQL parsing, replay, and UI flow
Configuration and version updates
Sequence Diagram(s)sequenceDiagram
participant Container as Container.vue
participant Service as GraphQLReplayService
participant ReplaySDK as sdk.replay
Container->>Service: createReplayFromRequest(rawRequest, targetUrl)
Service->>Service: parseConnection(targetUrl)
Service->>ReplaySDK: getCollections()
Service->>ReplaySDK: createCollection() or reuse existing
Service->>ReplaySDK: createSession({ raw, connectionInfo })
Service->>ReplaySDK: renameSession()
Service-->>Container: Result
sequenceDiagram
participant View as GraphQLViewMode.vue
participant Parser as parseHttpMessage / extractGraphQLOperation
participant HttpForge as HttpForge
participant Editor as CodeMirror
View->>Parser: parse raw request
Parser-->>View: HTTP + GraphQL data
View->>View: initializeData()
View->>HttpForge: reconstruct raw request
View->>Editor: dispatch updated content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/frontend/src/services/replay.ts`:
- Around line 13-22: The parseConnection function currently accepts any valid
URL including non-HTTP schemes like request: or mailto: which produce empty
hostnames, allowing invalid targets through. After calling URL.canParse and
creating the url object, add validation to ensure the url.protocol is either
http: or https: (exact match), and verify that url.hostname is not empty. Return
undefined if either validation fails, so only legitimate HTTP/HTTPS targets with
valid hostnames are accepted and processed into the ConnectionInfo object.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: df1e360c-3996-47f2-a35c-d8f24ba6a380
📒 Files selected for processing (3)
caido.config.tspackages/frontend/src/components/attacks/Container.vuepackages/frontend/src/services/replay.ts
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/frontend/src/utils/graphql.test.ts (1)
45-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for persistedQuery without
sha256Hash.Please add a negative case where
extensions.persistedQueryexists but lacks a valid hash, and assert it is not treated as GraphQL. This will lock behavior for the parser guard.Suggested test additions
describe("isGraphQLRequest", () => { + it("rejects persistedQuery payloads without a sha256Hash", () => { + const body = JSON.stringify({ + extensions: { persistedQuery: { version: 1 } }, + }); + expect(isGraphQLRequest(rawRequest("POST", body))).toBe(false); + });describe("extractGraphQLOperation", () => { + it("returns undefined when persistedQuery exists without a valid hash", () => { + const op = extractGraphQLOperation( + JSON.stringify({ extensions: { persistedQuery: {} } }), + ); + expect(op).toBeUndefined(); + });Also applies to: 91-102
🤖 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 `@packages/frontend/src/utils/graphql.test.ts` around lines 45 - 52, Add a new regression test case after the existing persisted query test to handle the negative scenario where extensions.persistedQuery exists but lacks the sha256Hash field. Create a test that constructs a request body with operationName, variables, and extensions.persistedQuery (but without the sha256Hash property), then call isGraphQLRequest with rawRequest and assert it returns false. This ensures the parser correctly rejects incomplete persisted query structures.packages/frontend/src/views/GraphQLViewMode.vue (1)
377-377: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHTTP version is hardcoded to
HTTP/1.1on reconstruction.
parseHttpMessageonly preserves the method, so the rebuilt request line forcesHTTP/1.1regardless of the original (e.g., HTTP/2) request line. For replay flows this is usually acceptable, but consider preserving the original version/request-line if downstream tooling relies on it.🤖 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 `@packages/frontend/src/views/GraphQLViewMode.vue` at line 377, The headerLines array construction in GraphQLViewMode.vue hardcodes HTTP/1.1 in the request line, ignoring the original HTTP version from the parsed request. Modify the headerLines assignment to extract and preserve the original HTTP version from the parsed message (accessed via the parsed object) instead of hardcoding HTTP/1.1, so that if the original request was HTTP/2 or another version, it will be reconstructed with the same version string. This ensures downstream tooling receives requests with the correct HTTP version that matches the original.
🤖 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 `@packages/frontend/src/utils/graphql.ts`:
- Around line 82-86: The condition checking `persistedQuery !== undefined` in
the query validation logic is too permissive and allows empty objects to pass as
valid, potentially causing false GraphQL positives. Instead of only checking
that persistedQuery is not undefined, add validation to ensure it contains a
valid hash value or other required properties. Modify the condition to check not
just for existence of persistedQuery but also verify that it has actual content
before marking the request as a valid GraphQLOperation.
In `@packages/frontend/src/views/GraphQLViewMode.vue`:
- Around line 141-148: The watch with immediate: true on getRawData is executing
synchronously during setup and calling initializeData(), which in turn calls
validateQuery() before validateQuery is declared later in the file, causing a
temporal dead zone error. Move the validateQuery function declaration (and any
other functions or helpers that initializeData depends on) to occur before the
watch definition, then remove the original validateQuery declaration that
appears later in the file. This ensures all dependencies are available when the
immediate watch callback executes.
- Around line 65-67: The isActuallyGraphQL computed property performs an exact
string match on parsedHttp.value?.method === "POST", which fails for HTTP
methods with lowercase values like "post" or "Post". The registration predicate
isGraphQLRequest normalizes the method using toUpperCase(), allowing lowercase
methods to pass registration but then failing this check, causing a mismatch.
Fix this by normalizing the method comparison in isActuallyGraphQL to use
parsedHttp.value?.method?.toUpperCase() === "POST" to ensure consistency with
how isGraphQLRequest validates the method.
---
Nitpick comments:
In `@packages/frontend/src/utils/graphql.test.ts`:
- Around line 45-52: Add a new regression test case after the existing persisted
query test to handle the negative scenario where extensions.persistedQuery
exists but lacks the sha256Hash field. Create a test that constructs a request
body with operationName, variables, and extensions.persistedQuery (but without
the sha256Hash property), then call isGraphQLRequest with rawRequest and assert
it returns false. This ensures the parser correctly rejects incomplete persisted
query structures.
In `@packages/frontend/src/views/GraphQLViewMode.vue`:
- Line 377: The headerLines array construction in GraphQLViewMode.vue hardcodes
HTTP/1.1 in the request line, ignoring the original HTTP version from the parsed
request. Modify the headerLines assignment to extract and preserve the original
HTTP version from the parsed message (accessed via the parsed object) instead of
hardcoding HTTP/1.1, so that if the original request was HTTP/2 or another
version, it will be reconstructed with the same version string. This ensures
downstream tooling receives requests with the correct HTTP version that matches
the original.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 011fa418-f0d1-44f0-815d-52b40d9fe25f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
README.mdpackages/backend/package.jsonpackages/frontend/package.jsonpackages/frontend/src/index.tspackages/frontend/src/services/replay.tspackages/frontend/src/utils/graphql.test.tspackages/frontend/src/utils/graphql.tspackages/frontend/src/views/GraphQLViewMode.vue
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/frontend/src/services/replay.ts
…-query add introspection query in view mode
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
2 participants
This PR fixes:
Summary by CodeRabbit