Skip to content

refactor: consolidate duplicated security HTTP client logic#126

Merged
deekshithgowda85 merged 2 commits into
deekshithgowda85:prodfrom
trivikramkalagi91-commits:issue-119-http-client-consolidation
Jun 21, 2026
Merged

refactor: consolidate duplicated security HTTP client logic#126
deekshithgowda85 merged 2 commits into
deekshithgowda85:prodfrom
trivikramkalagi91-commits:issue-119-http-client-consolidation

Conversation

@trivikramkalagi91-commits

@trivikramkalagi91-commits trivikramkalagi91-commits commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Summary

This PR addresses Issue #119 by consolidating duplicated HTTP request execution logic used by the security scanning modules.

A new shared utility (lib/utils/securityHttpClient.ts) has been introduced to centralize common functionality such as request execution, timeout handling, latency measurement, response header collection, body truncation, Set-Cookie extraction, and error mapping.

The existing public APIs remain unchanged through compatibility wrappers, ensuring no consumer code needs to be modified.

Related Issue

Closes: #119

What Changed

  • Added lib/utils/securityHttpClient.ts as a shared internal implementation for security HTTP requests
  • Refactored lib/attack-pipeline/utils/httpClient.ts to use the shared implementation while preserving HttpSession and existing behavior
  • Refactored lib/security-agent/utils/http-client.ts to use the shared implementation while preserving its public API and defaults
  • Preserved redirect behavior, timeout handling, latency calculation, response mapping, body truncation limits, and error handling
  • Reduced duplicated request execution code between attack-pipeline and security-agent modules

Verification

  • I ran pnpm lint (equivalent ESLint validation completed successfully)
  • I manually verified the affected page, API route, or sandbox flow
  • I checked that no secrets or hardcoded credentials were added

Additional Validation

  • TypeScript compilation completed successfully
  • Existing imports remain unchanged
  • HttpSession behavior preserved
  • Cookie persistence preserved
  • Redirect handling preserved
  • Body truncation limits preserved
  • Error response behavior preserved

Checklist

  • My branch name follows the repository convention
  • My commits use Conventional Commits
  • I updated docs if the behavior or workflow changed
  • I kept the change scoped to one issue
  • I am ready for review

Summary by CodeRabbit

  • Refactor
    • Consolidated HTTP request execution for the security scanner and related components into a new shared utility.
    • Centralized handling for timeouts/abort behavior, response metadata (status, headers, final URL/redirect info, latency), cookie collection, body truncation, and standardized error shaping.
    • Updated existing HTTP clients to delegate execution to the shared implementation while preserving their outward-facing behavior and signatures.

@vercel

vercel Bot commented Jun 14, 2026

Copy link
Copy Markdown

@trivikramkalagi91-commits is attempting to deploy a commit to the Deekshith Gowda HS's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

A new shared module lib/utils/securityHttpClient.ts is introduced, exporting SharedRequestOptions, SharedResponse, and executeSecurityRequest. Both lib/attack-pipeline/utils/httpClient.ts and lib/security-agent/utils/http-client.ts are refactored to import and delegate to executeSecurityRequest, removing their inline fetch, timeout, error-handling, and response-parsing logic.

Changes

Shared HTTP client consolidation

Layer / File(s) Summary
Shared executeSecurityRequest utility
lib/utils/securityHttpClient.ts
New module exporting SharedRequestOptions and SharedResponse interfaces plus executeSecurityRequest, which wraps fetch with AbortSignal.timeout, latency measurement, header/cookie normalization via getSetCookie(), body truncation, redirected/finalUrl reporting, and standardized error-to-zero-status response mapping.
Attack-pipeline and security-agent HTTP client delegation
lib/attack-pipeline/utils/httpClient.ts, lib/security-agent/utils/http-client.ts
Both clients import executeSecurityRequest and replace their inline fetch/abort/timeout/error-handling with a single delegating call. attack-pipeline passes a fixed 20_000 truncate limit and maps followRedirects to redirect mode; security-agent defaults redirect to "manual" and conditionally assigns result.error from the shared response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

quality:clean, type:design

Poem

🐇 Hop, hop, hooray — no more copy and paste!
One shared HTTP helper, no duplicate waste.
executeSecurityRequest now leads the way,
With cookies and latency neatly in play.
The burrow is tidy, the pipelines aligned —
A cleaner codebase is what we designed! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: consolidation of duplicated HTTP client logic into a shared utility module.
Linked Issues check ✅ Passed The PR fully addresses Issue #119 by extracting shared HTTP client logic into lib/utils/securityHttpClient.ts and refactoring both attack-pipeline and security-agent modules to use it.
Out of Scope Changes check ✅ Passed All changes are scoped to the consolidation objective: new shared utility module, refactored httpClient modules, and no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@lib/utils/securityHttpClient.ts`:
- Around line 54-55: The fallback handling for the set-cookie header at line 55
in securityHttpClient.ts pushes the entire header value as a single string, but
the downstream parseCookies function expects one cookie per array entry. When a
set-cookie header contains multiple cookies (typically comma-separated), they
need to be split into individual entries. Modify the code to split the
set-cookie header value by comma and push each cookie as a separate entry into
the setCookies array instead of pushing the entire header as one string.
🪄 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: 7050ad9d-d455-4990-9481-70052211970f

📥 Commits

Reviewing files that changed from the base of the PR and between bbb3b64 and 95666de.

📒 Files selected for processing (3)
  • lib/attack-pipeline/utils/httpClient.ts
  • lib/security-agent/utils/http-client.ts
  • lib/utils/securityHttpClient.ts

Comment thread lib/utils/securityHttpClient.ts Outdated
@trivikramkalagi91-commits

Copy link
Copy Markdown
Contributor Author

@deekshithgowda85 please review thr PR and it is ready for merge

@trivikramkalagi91-commits

Copy link
Copy Markdown
Contributor Author

@deekshithgowda85 itsbeen 4 days can u please look over the PR and merge as there is no base confl_ct with base branch
_

@trivikramkalagi91-commits

Copy link
Copy Markdown
Contributor Author

@deekshithgowda85 i have solved the issue suggested by coderabbitai please review as it is ready to merge

@trivikramkalagi91-commits

Copy link
Copy Markdown
Contributor Author

@deekshithgowda85 it has been almost 1 week and i have solved the issue suggested by coderabbitai please review as it is ready to merge

@trivikramkalagi91-commits

Copy link
Copy Markdown
Contributor Author

/coderabbitai review

@deekshithgowda85 deekshithgowda85 merged commit a6bc3a0 into deekshithgowda85:prod Jun 21, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Consolidate duplicate HTTP client utilities into a shared module

2 participants