feat: add Caido web proxy for HTTP traffic interception & replay#305
Open
feat: add Caido web proxy for HTTP traffic interception & replay#305
Conversation
Integrates Caido web security proxy into the agent workflow, enabling automatic interception, inspection, and replay of all HTTP/HTTPS traffic during pentesting. Key changes: - Docker: install caido-cli, auto-start via docker-entrypoint.sh - 7 proxy tools: list_requests, view_request, send_request, repeat_request, scope_rules, list_sitemap, view_sitemap_entry - proxy-manager.ts: TypeScript port of Strix's proxy_manager.py, executes Caido GraphQL via curl on sandbox (works for desktop + remote-connection) - ensureCaido: auto-installs caido-cli if missing, starts daemon, authenticates, creates project — with Promise-based lock to prevent parallel races - run-terminal-cmd: injects HTTP_PROXY env vars so all commands route through Caido - System prompt: proxy_interception section (conditional on caido_enabled setting) - Settings: simple toggle to disable Caido proxy per user - Sidebar: formatted proxy tool output with tabular requests, color-coded status - Auto-recovery: detects broken Caido DB, kills and restarts automatically Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Forces recreation of existing sandboxes so they pick up the new Docker image with caido-cli and the auto-start entrypoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Migrate from deprecated Template.build(template, { alias }) to
Template.build(template, name, options) signature
- Add skipCache() to template builder so E2B always pulls latest
Docker image instead of using stale cached layers
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Remove E2B exclusion from proxy tools and HTTP_PROXY env var injection - Add setStartCmd to E2B template: runs docker-entrypoint.sh with sudo on sandbox boot, waits for port 48080 (Caido ready) before accepting commands - Remove isE2BSandboxPreference from ToolContext (no longer needed) - Use nohup+disown for caido-cli background process to survive shell exit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Caido-backed proxy tooling and UI integration: new proxy tools and manager, sandbox/Caido orchestration and env plumbing, sidebar/UI rendering for proxy executions, user preference support ( Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent (AI)
participant Tool as Proxy Tool
participant Sandbox as Sandbox Env
participant Caido as caido-cli / Caido Service
Agent->>Tool: invoke proxy tool (e.g., send_request)
Tool->>Sandbox: ensureCaido(context)
alt Caido already healthy
Sandbox->>Sandbox: use cached token / proceed
else needs setup/start
Sandbox->>Caido: run health/auth/project start script (may start caido-cli)
Caido-->>Sandbox: readiness + token
Sandbox->>Sandbox: store token (/tmp/caido-token)
end
Tool->>Sandbox: run GraphQL or proxied curl (with HTTP_PROXY env)
Sandbox->>Caido: GraphQL / proxied request
Caido-->>Sandbox: response JSON / proxied response
Sandbox-->>Tool: parsed result
Tool-->>Agent: return { result }
sequenceDiagram
participant UI as MessagePartHandler / ProxyToolHandler
participant User as Local User
participant Sidebar as ComputerSidebar
participant Exec as toolExecutions (live)
UI->>UI: render ProxyToolBlock (shimmer or clickable)
User->>UI: click / Enter on ToolBlock
UI->>Sidebar: open sidebar with SidebarProxy content
Sidebar->>Exec: resolve live execution by toolCallId (if streaming)
Sidebar->>Sidebar: display command, output, status (streaming/ready)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
rossmanko
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
convex/userCustomization.ts (1)
74-99:⚠️ Potential issue | 🟠 MajorDo not reset
caido_enabledduring unrelated customization updates.
caido_enabled: args.caido_enabled ?? truewill flip an existingfalseback totruewhenever this arg is omitted in an update payload.💡 Suggested fix
const existing = await ctx.db .query("user_customization") .withIndex("by_user_id", (q) => q.eq("user_id", identity.subject)) .first(); + const resolvedCaidoEnabled = + args.caido_enabled !== undefined + ? args.caido_enabled + : existing?.caido_enabled ?? true; + const customizationData = { user_id: identity.subject, nickname: args.nickname?.trim() || undefined, occupation: args.occupation?.trim() || undefined, personality: args.personality?.trim() || undefined, traits: args.traits?.trim() || undefined, additional_info: args.additional_info?.trim() || undefined, include_memory_entries: args.include_memory_entries !== undefined ? args.include_memory_entries : true, // Default to enabled guardrails_config: args.guardrails_config?.trim() || undefined, - caido_enabled: args.caido_enabled ?? true, + caido_enabled: resolvedCaidoEnabled, extra_usage_enabled: args.extra_usage_enabled !== undefined ? args.extra_usage_enabled : false, // Default to disabled updated_at: Date.now(), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/userCustomization.ts` around lines 74 - 99, The caido_enabled field in the customizationData object is unconditionally defaulting to true via "caido_enabled: args.caido_enabled ?? true", which will overwrite an existing false when args.caido_enabled is omitted; fix by only assigning caido_enabled from args when it is explicitly provided and otherwise preserve the existing value (when updating) or use the intended default (when creating). Locate the customizationData construction (variable customizationData) and the existing lookup (const existing = await ctx.db.query("user_customization")...) and change the caido_enabled assignment to: if args.caido_enabled is defined use it, else if existing use existing.caido_enabled, else use the creation-time default.
🧹 Nitpick comments (10)
app/components/MessagePartHandler.tsx (1)
218-257: Consider consolidating proxy tool cases with a lookup map.These new branches are correct, but a
Record<partType, toolName>would reduce repetition and make adding future proxy tools safer.♻️ Optional refactor sketch
+ const proxyToolMap: Record<string, ProxyToolName> = { + "tool-list_requests": "list_requests", + "tool-view_request": "view_request", + "tool-send_request": "send_request", + "tool-repeat_request": "repeat_request", + "tool-scope_rules": "scope_rules", + "tool-list_sitemap": "list_sitemap", + "tool-view_sitemap_entry": "view_sitemap_entry", + }; switch (part.type) { + case "tool-list_requests": + case "tool-view_request": + case "tool-send_request": + case "tool-repeat_request": + case "tool-scope_rules": + case "tool-list_sitemap": + case "tool-view_sitemap_entry": + return ( + <ProxyToolHandler + part={part} + status={status} + toolName={proxyToolMap[part.type]} + /> + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/MessagePartHandler.tsx` around lines 218 - 257, The switch contains many repetitive branches that all render ProxyToolHandler; replace them with a lookup map (e.g. const proxyToolMap: Record<string,string>) mapping the part types like "tool-list_requests", "tool-view_request", etc. to their toolName values, then in MessagePartHandler.tsx compute const toolName = proxyToolMap[part.type] and, if present, return <ProxyToolHandler part={part} status={status} toolName={toolName} />; this removes duplicate case blocks and centralizes future additions while keeping ProxyToolHandler as the single render target.app/share/[shareId]/components/SharedMessagePartHandler.tsx (1)
45-48: Remove unused importPROXY_ACTION_LABELS.
PROXY_ACTION_LABELSis imported but not used in this file - onlyPROXY_COMPLETED_LABELSis referenced at line 648.🧹 Proposed fix
-import { - PROXY_ACTION_LABELS, - PROXY_COMPLETED_LABELS, -} from "@/app/components/tools/ProxyToolHandler"; +import { PROXY_COMPLETED_LABELS } from "@/app/components/tools/ProxyToolHandler";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/share/`[shareId]/components/SharedMessagePartHandler.tsx around lines 45 - 48, Remove the unused import PROXY_ACTION_LABELS from the import list in SharedMessagePartHandler.tsx so only PROXY_COMPLETED_LABELS is imported; locate the import statement that currently reads import { PROXY_ACTION_LABELS, PROXY_COMPLETED_LABELS } from "@/app/components/tools/ProxyToolHandler" and delete the PROXY_ACTION_LABELS identifier (leaving PROXY_COMPLETED_LABELS) to clean up the unused symbol.lib/utils/sidebar-utils.ts (1)
458-466: Consider extracting proxy tool types to a shared constant.This array of proxy tool type strings is duplicated in
SharedMessagePartHandler.tsx(lines 179-186). Extracting it to a shared constant (e.g., inProxyToolHandler.tsxalongside the label maps) would improve maintainability.♻️ Example refactor
In
app/components/tools/ProxyToolHandler.tsx:export const PROXY_TOOL_TYPES = [ "tool-list_requests", "tool-view_request", "tool-send_request", "tool-repeat_request", "tool-scope_rules", "tool-list_sitemap", "tool-view_sitemap_entry", ] as const;Then import and use in both files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils/sidebar-utils.ts` around lines 458 - 466, The proxyToolTypes array is duplicated across sidebar-utils.ts (proxyToolTypes) and SharedMessagePartHandler.tsx; extract it into a shared exported constant (e.g., PROXY_TOOL_TYPES) inside ProxyToolHandler.tsx alongside the label maps, export it (as readonly/const), and update both files to import and use PROXY_TOOL_TYPES instead of their local proxyToolTypes definitions to avoid duplication.docker/Dockerfile (1)
214-228: Improve Caido CLI installation: use official latest endpoint and add error handling.The current approach fetches the latest version via GitHub API with basic grep parsing, which is fragile and lacks error handling. Per Caido's official documentation, use the
caido.download/releases/latestendpoint instead, which provides structured JSON and is the recommended approach. Additionally, add error handling for failed downloads and usecurl -Lto follow redirects (required by caido.download).Consider one of these alternatives:
Use the official latest endpoint (simplest):
curl -s https://caido.download/releases/latest \ | jq -r '.links[] | select(.kind=="cli" and .platform=="linux-'${CAIDO_ARCH}'") | .link' \ | xargs -I {} curl -L -o /tmp/caido-cli.tar.gz {}Pin to a specific version (most reproducible):
CAIDO_VERSION="v0.55.3" curl -L -o /tmp/caido-cli.tar.gz \ "https://caido.download/releases/${CAIDO_VERSION}/caido-cli-${CAIDO_VERSION}-linux-${CAIDO_ARCH}.tar.gz"Either approach is preferable to fetching from GitHub API and parsing with grep.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile` around lines 214 - 228, Replace the fragile GitHub API + grep flow that sets CAIDO_VERSION and downloads caido-cli with the official caido.download approach: derive CAIDO_ARCH as before, then either query https://caido.download/releases/latest (use curl -L and parse JSON with jq to pick the CLI link matching linux-${CAIDO_ARCH}) or set a pinned CAIDO_VERSION and build the direct caido.download URL; ensure the download uses curl -L (follow redirects), verify the download succeeded (non‑zero exit or missing /tmp/caido-cli.tar.gz should abort), check tar extraction and chmod/mv return codes and fail early with a clear error message if any step (download, tar, mv) fails, keeping the rest of the install steps that set executable and move to /usr/local/bin/caido-cli and create /app/certs.lib/ai/tools/proxy-tool.ts (1)
156-169: Consider adding URL validation.The
urlfield usesz.string()without validation. Considerz.string().url()to catch malformed URLs early, though this may intentionally allow non-standard URLs for testing purposes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/proxy-tool.ts` around lines 156 - 169, The inputSchema currently defines url as z.string() which allows malformed URLs; update the url validator in the inputSchema to use z.string().url() (or z.string().url().optional() if you expect nullable/optional values) so malformed targets are rejected early; locate the inputSchema object in proxy-tool.ts (the url property inside inputSchema) and replace the z.string() for url with z.string().url() and adjust any calling code/tests if they relied on permissive strings.app/components/tools/ProxyToolHandler.tsx (1)
8-13: Consider adding a type forpartprop.Using
anyfor thepartprop loses type safety. Consider defining aProxyToolPartinterface or using a generic tool part type if one exists in the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/tools/ProxyToolHandler.tsx` around lines 8 - 13, The prop type uses any for `part` in `ProxyToolHandlerProps`, losing type safety; define a concrete interface (e.g., `ProxyToolPart`) or reuse an existing tool-part type and replace `part: any` with `part: ProxyToolPart` in `ProxyToolHandlerProps`, then update any consumers of `ProxyToolHandler` (and related functions/components) to conform to the new shape (add optional/required fields, unions, or generics as needed) and adjust imports/exports so `ProxyToolPart` is available where `ProxyToolHandlerProps` is declared.lib/ai/tools/utils/proxy-manager.ts (3)
780-783: Protocol detection heuristic may be incorrect.The check
host.includes(":443")assumes port 443 means HTTPS, but the Host header might not include the port, and HTTPS can run on non-443 ports. Consider using theisTlsfield from the original request data if available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/utils/proxy-manager.ts` around lines 780 - 783, The protocol detection using host.includes(":443") is unreliable; update the logic in proxy-manager.ts where fullUrl is built (the code that sets protocol and fullUrl, referencing variables host, urlPath, and modifications.url) to prefer the original request's TLS indicator (e.g., an isTls field on the original request object) to decide "https" vs "http", falling back to the existing host port heuristic only if isTls is undefined; ensure the rest of the code still uses modifications.url when present.
121-149: GitHub API rate limiting could cause silent failures.The
install_caido_clifunction fetches the latest version from GitHub API, which may be rate-limited (60 requests/hour unauthenticated). Consider caching the version or providing a fallback version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/utils/proxy-manager.ts` around lines 121 - 149, The install_caido_cli function (inside the installFn string) can fail silently when the GitHub API returns empty CAIDO_VERSION due to rate limits; update the logic to: if CAIDO_VERSION is empty, try (in order) an environment override (e.g., CAIDO_CLI_VERSION), then a small local cache file (e.g., ~/.cache/caido_cli_version), and finally fall back to a DEFAULT_CAIDO_VERSION constant; ensure that after a successful retrieval/install you write the resolved version to the cache so future runs avoid the API, and log a clear warning when falling back so failures are visible.
1000-1004: Client-side pagination may be inefficient for large sitemaps.The function fetches all nodes from GraphQL and then slices for pagination. For very large sitemaps, this could be memory-intensive. Consider if the Caido API supports server-side pagination.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/utils/proxy-manager.ts` around lines 1000 - 1004, The current pagination builds allNodes then slices it (variables allNodes, pageNodes, totalCount, totalPages) which is memory-inefficient; update the GraphQL request in proxy-manager.ts to use server-side pagination instead (e.g., add limit/offset or Relay-style first/after to the query) so the API returns only the needed page, compute totalCount from the returned count field, and remove the allNodes.slice logic—adjust downstream code to use the returned edges/nodes and pageInfo or computed totalPages accordingly.docker/docker-entrypoint.sh (1)
105-107: Consider failing the entrypoint if authentication is critical.The script warns but continues if Caido authentication fails after 5 attempts. If Caido is essential for the container's purpose, consider
exit 1after the warning to prevent a partially functional container.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/docker-entrypoint.sh` around lines 105 - 107, The entrypoint currently only warns when the TOKEN variable is empty (in docker-entrypoint.sh the if [ -z "$TOKEN" ] check) which allows the container to continue without Caido auth; if authentication is critical, change this branch to terminate the process (call exit 1) after logging the warning so the container fails fast when TOKEN is not set; update any related documentation or startup logic that expects a graceful continue if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/AgentsTab.tsx`:
- Around line 193-202: The onCheckedChange handler for the Caido toggle allows
concurrent saveCustomization calls causing race conditions; add an in-flight
guard (eg. isSavingCaido state) around the handler in the AgentsTab component so
if isSavingCaido is true new toggles are ignored (or disabled in the UI) until
the current saveCustomization({ caido_enabled: ... }) promise resolves/rejects,
set isSavingCaido = true before awaiting saveCustomization and reset it in
finally, and ensure toast logic runs only for the latest completed save to avoid
out-of-order state updates.
In `@lib/ai/tools/utils/caido-proxy.ts`:
- Around line 6-20: The buildCaidoProxyEnvVars function currently types its
parameter as typeof CAIDO_DEFAULTS and unconditionally sets global TLS-disable
env vars (NODE_TLS_REJECT_UNAUTHORIZED, PYTHONHTTPSVERIFY, REQUESTS_CA_BUNDLE);
change the parameter to accept a looser runtime-friendly type (e.g.,
Partial<Record<"host"|"port", string>> or a small interface instead of typeof
CAIDO_DEFAULTS) so callers can pass plain objects, and stop setting global
TLS-disable keys by default — remove NODE_TLS_REJECT_UNAUTHORIZED,
PYTHONHTTPSVERIFY and REQUESTS_CA_BUNDLE from the default returned map and
instead accept an explicit option (e.g., allowInsecure: boolean) or an explicit
extraEnv parameter to opt-in to adding those variables when the caller
intentionally requests insecure behavior; keep returning both uppercase and
lowercase proxy keys (HTTP_PROXY/HTTPS_PROXY/ALL_PROXY and
http_proxy/https_proxy) and ensure proxyUrl construction still uses
config.host/config.port (function: buildCaidoProxyEnvVars, constant:
CAIDO_DEFAULTS, env names: NODE_TLS_REJECT_UNAUTHORIZED, PYTHONHTTPSVERIFY,
REQUESTS_CA_BUNDLE).
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Around line 665-679: The curl command builds a single interpolated shell
string (variables headerArgs, bodyArg, cmd) which allows header values and url
to inject shell metacharacters; fix by not concatenating a raw shell string:
construct a argv array of curl arguments and run curl via
child_process.spawn/execFile (or use a safe HTTP client), pass each header as
separate '-H', pass body via '--data-raw' as an arg (or pipe binary data) and
validate/percent-encode the URL and header names/values before adding them;
remove any direct interpolation into a single quoted shell command so no
unescaped user input reaches the shell.
- Around line 500-542: searchContent currently constructs a RegExp from
user-controlled pattern (pattern) which risks ReDoS; to fix, validate and
constrain the input before creating regex: enforce a maximum pattern length
(e.g., 200 chars), reject or escape suspicious constructs (or run through a
regex-safety validator lib) and return an error for unsafe patterns, and wrap
the RegExp.exec loop in a time-bounded/iteration-bounded guard (already limits
to 20 matches but add an overall operation timeout or max iterations counter) so
the regex creation/execution (regex and while loop over matches) cannot hang the
process; make these checks inside searchContent before new RegExp(...) and
surface a clear error when a pattern is refused.
---
Outside diff comments:
In `@convex/userCustomization.ts`:
- Around line 74-99: The caido_enabled field in the customizationData object is
unconditionally defaulting to true via "caido_enabled: args.caido_enabled ??
true", which will overwrite an existing false when args.caido_enabled is
omitted; fix by only assigning caido_enabled from args when it is explicitly
provided and otherwise preserve the existing value (when updating) or use the
intended default (when creating). Locate the customizationData construction
(variable customizationData) and the existing lookup (const existing = await
ctx.db.query("user_customization")...) and change the caido_enabled assignment
to: if args.caido_enabled is defined use it, else if existing use
existing.caido_enabled, else use the creation-time default.
---
Nitpick comments:
In `@app/components/MessagePartHandler.tsx`:
- Around line 218-257: The switch contains many repetitive branches that all
render ProxyToolHandler; replace them with a lookup map (e.g. const
proxyToolMap: Record<string,string>) mapping the part types like
"tool-list_requests", "tool-view_request", etc. to their toolName values, then
in MessagePartHandler.tsx compute const toolName = proxyToolMap[part.type] and,
if present, return <ProxyToolHandler part={part} status={status}
toolName={toolName} />; this removes duplicate case blocks and centralizes
future additions while keeping ProxyToolHandler as the single render target.
In `@app/components/tools/ProxyToolHandler.tsx`:
- Around line 8-13: The prop type uses any for `part` in
`ProxyToolHandlerProps`, losing type safety; define a concrete interface (e.g.,
`ProxyToolPart`) or reuse an existing tool-part type and replace `part: any`
with `part: ProxyToolPart` in `ProxyToolHandlerProps`, then update any consumers
of `ProxyToolHandler` (and related functions/components) to conform to the new
shape (add optional/required fields, unions, or generics as needed) and adjust
imports/exports so `ProxyToolPart` is available where `ProxyToolHandlerProps` is
declared.
In `@app/share/`[shareId]/components/SharedMessagePartHandler.tsx:
- Around line 45-48: Remove the unused import PROXY_ACTION_LABELS from the
import list in SharedMessagePartHandler.tsx so only PROXY_COMPLETED_LABELS is
imported; locate the import statement that currently reads import {
PROXY_ACTION_LABELS, PROXY_COMPLETED_LABELS } from
"@/app/components/tools/ProxyToolHandler" and delete the PROXY_ACTION_LABELS
identifier (leaving PROXY_COMPLETED_LABELS) to clean up the unused symbol.
In `@docker/docker-entrypoint.sh`:
- Around line 105-107: The entrypoint currently only warns when the TOKEN
variable is empty (in docker-entrypoint.sh the if [ -z "$TOKEN" ] check) which
allows the container to continue without Caido auth; if authentication is
critical, change this branch to terminate the process (call exit 1) after
logging the warning so the container fails fast when TOKEN is not set; update
any related documentation or startup logic that expects a graceful continue if
needed.
In `@docker/Dockerfile`:
- Around line 214-228: Replace the fragile GitHub API + grep flow that sets
CAIDO_VERSION and downloads caido-cli with the official caido.download approach:
derive CAIDO_ARCH as before, then either query
https://caido.download/releases/latest (use curl -L and parse JSON with jq to
pick the CLI link matching linux-${CAIDO_ARCH}) or set a pinned CAIDO_VERSION
and build the direct caido.download URL; ensure the download uses curl -L
(follow redirects), verify the download succeeded (non‑zero exit or missing
/tmp/caido-cli.tar.gz should abort), check tar extraction and chmod/mv return
codes and fail early with a clear error message if any step (download, tar, mv)
fails, keeping the rest of the install steps that set executable and move to
/usr/local/bin/caido-cli and create /app/certs.
In `@lib/ai/tools/proxy-tool.ts`:
- Around line 156-169: The inputSchema currently defines url as z.string() which
allows malformed URLs; update the url validator in the inputSchema to use
z.string().url() (or z.string().url().optional() if you expect nullable/optional
values) so malformed targets are rejected early; locate the inputSchema object
in proxy-tool.ts (the url property inside inputSchema) and replace the
z.string() for url with z.string().url() and adjust any calling code/tests if
they relied on permissive strings.
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Around line 780-783: The protocol detection using host.includes(":443") is
unreliable; update the logic in proxy-manager.ts where fullUrl is built (the
code that sets protocol and fullUrl, referencing variables host, urlPath, and
modifications.url) to prefer the original request's TLS indicator (e.g., an
isTls field on the original request object) to decide "https" vs "http", falling
back to the existing host port heuristic only if isTls is undefined; ensure the
rest of the code still uses modifications.url when present.
- Around line 121-149: The install_caido_cli function (inside the installFn
string) can fail silently when the GitHub API returns empty CAIDO_VERSION due to
rate limits; update the logic to: if CAIDO_VERSION is empty, try (in order) an
environment override (e.g., CAIDO_CLI_VERSION), then a small local cache file
(e.g., ~/.cache/caido_cli_version), and finally fall back to a
DEFAULT_CAIDO_VERSION constant; ensure that after a successful retrieval/install
you write the resolved version to the cache so future runs avoid the API, and
log a clear warning when falling back so failures are visible.
- Around line 1000-1004: The current pagination builds allNodes then slices it
(variables allNodes, pageNodes, totalCount, totalPages) which is
memory-inefficient; update the GraphQL request in proxy-manager.ts to use
server-side pagination instead (e.g., add limit/offset or Relay-style
first/after to the query) so the API returns only the needed page, compute
totalCount from the returned count field, and remove the allNodes.slice
logic—adjust downstream code to use the returned edges/nodes and pageInfo or
computed totalPages accordingly.
In `@lib/utils/sidebar-utils.ts`:
- Around line 458-466: The proxyToolTypes array is duplicated across
sidebar-utils.ts (proxyToolTypes) and SharedMessagePartHandler.tsx; extract it
into a shared exported constant (e.g., PROXY_TOOL_TYPES) inside
ProxyToolHandler.tsx alongside the label maps, export it (as readonly/const),
and update both files to import and use PROXY_TOOL_TYPES instead of their local
proxyToolTypes definitions to avoid duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f959d66d-be03-4662-b66f-38fb21d683d4
📒 Files selected for processing (28)
app/components/AgentsTab.tsxapp/components/ComputerSidebar.tsxapp/components/MessagePartHandler.tsxapp/components/computer-sidebar-utils.tsxapp/components/tools/ProxyToolHandler.tsxapp/hooks/useSidebarNavigation.tsapp/share/[shareId]/components/SharedMessagePartHandler.tsxconvex/schema.tsconvex/userCustomization.tsdocker/Dockerfiledocker/docker-entrypoint.she2b/build.dev.tse2b/build.prod.tse2b/template.tslib/ai/tools/index.tslib/ai/tools/proxy-tool.tslib/ai/tools/run-terminal-cmd.tslib/ai/tools/utils/caido-proxy.tslib/ai/tools/utils/proxy-manager.tslib/ai/tools/utils/sandbox-command-options.tslib/ai/tools/utils/sandbox.tslib/api/chat-handler.tslib/system-prompt.tslib/utils/sidebar-utils.tssrc/trigger/agent-task.tstypes/agent.tstypes/chat.tstypes/user.ts
- Delete docker-entrypoint.sh (ensureCaido handles all sandbox types) - Remove ENTRYPOINT from Dockerfile - Add --ui-domain flag for E2B so Caido UI is accessible via public URL - Set CAIDO_UI_URL env var on sandbox for AI to share with users - Use persistent "hackerai" project (reused across restarts) - Start caido-cli via sandbox background command (survives on E2B) - Strip debug logging, remove unused getCaidoUiUrl export - Add skipCache() to E2B template builder - Update system prompt: AI tells users to run echo $CAIDO_UI_URL Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
lib/ai/tools/utils/proxy-manager.ts (2)
708-723:⚠️ Potential issue | 🔴 CriticalStop interpolating request fields into a raw shell command.
Headers/method/URL are still shell-injectable in the current
curlcommand construction.🔒 Suggested escaping/validation baseline
+ const shQuote = (s: string) => `'${s.replace(/'/g, `'\\''`)}'`; + const safeMethod = method.toUpperCase(); + if (!/^[A-Z]+$/.test(safeMethod)) { + return { error: "Invalid HTTP method" }; + } + const safeTimeout = Number.isFinite(timeout) + ? Math.max(1, Math.min(timeout, 120)) + : 30; + const headerArgs = Object.entries(headers) - .map(([k, v]) => `-H "${k}: ${v}"`) + .map(([k, v]) => `-H ${shQuote(`${k}: ${v}`)}`) .join(" "); - const bodyArg = body ? `--data-raw '${body.replace(/'/g, "'\\''")}'` : ""; + const bodyArg = body ? `--data-raw ${shQuote(body)}` : ""; const cmd = [ `curl -siL --proxy ${proxyUrl} --insecure`, - `-X ${method.toUpperCase()}`, + `-X ${shQuote(safeMethod)}`, headerArgs, bodyArg, - `--max-time ${timeout}`, + `--max-time ${safeTimeout}`, `-w '\n__CURL_META__{"status":%{http_code},"time_ms":%{time_total},"url_effective":"%{url_effective}"}'`, - `"${url}"`, + shQuote(url), ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/utils/proxy-manager.ts` around lines 708 - 723, The current construction of the curl command by interpolating headerArgs, bodyArg, method, url and proxyUrl into a single shell string (variables headerArgs, bodyArg, cmd) is vulnerable to shell injection; change this to build an argv array and call child_process.spawn/execFile (no shell) so each header is a separate argument (e.g., push '-H', `${k}: ${v}` for each header), add body via ['--data-raw', body] (or pipe body to stdin) instead of embedding in a quoted string, and validate/strictly whitelist the HTTP method and sanitize the URL and proxyUrl before using them; update the code that creates cmd to produce an array of args and switch the invocation to spawn/execFile to eliminate shell interpolation.
549-559:⚠️ Potential issue | 🟠 MajorRegex search still allows ReDoS patterns from user input.
new RegExp(pattern, "gim")on untrusted input can cause catastrophic backtracking.🛡️ Suggested guardrails
function searchContent( requestData: unknown, content: string, pattern: string, ): Record<string, unknown> { try { + if (pattern.length > 300) { + return { error: "Pattern too long (max 300 chars)" }; + } + const startedAt = Date.now(); const regex = new RegExp(pattern, "gim"); @@ - while ((m = regex.exec(content)) !== null && matches.length < 20) { + while ((m = regex.exec(content)) !== null && matches.length < 20) { + if (Date.now() - startedAt > 150) { + return { error: "Regex search timed out" }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/utils/proxy-manager.ts` around lines 549 - 559, The code currently constructs a RegExp from untrusted input via new RegExp(pattern, "gim") (symbols: pattern, regex, regex.exec, content, matches) which allows ReDoS; fix by validating or sanitizing the incoming pattern before constructing a RegExp: run a safety check (e.g., using a safe-regex/regexpu or equivalent heuristic) and reject or escape the pattern if it is unsafe, falling back to a safe substring search (indexOf / includes) on content; ensure regex construction is wrapped in a try/catch and only proceed with regex.exec if the safety check passes to prevent catastrophic backtracking.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/Dockerfile`:
- Around line 219-227: Replace the current dynamic "releases/latest" workflow
(CAIDO_VERSION, wget of caido-cli-${CAIDO_VERSION}-linux-${CAIDO_ARCH}.tar.gz
and immediate tar/mv) with a pinned-version install that downloads and verifies
the published SHA-256 before extracting and installing: set a fixed
CAIDO_VERSION, fetch the corresponding .sha256 sidecar for
caido-cli-${CAIDO_VERSION}-linux-${CAIDO_ARCH}.tar.gz, verify the tarball
checksum (fail the build on mismatch), only then extract /tmp/caido-cli and mv
it to /usr/local/bin/caido-cli, and remove artifacts; ensure the verification
step runs before chmod/mv to guarantee supply-chain integrity.
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Line 1056: The range label `"${skipCount + 1}-${Math.min(skipCount + pageSize,
totalCount)} of ${totalCount}"` produces `1-0 of 0` when totalCount is 0; update
the logic that sets the showing value (the showing field using skipCount,
pageSize, totalCount) to special-case totalCount === 0 and set showing to `"0 of
0"`, otherwise compute start = skipCount + 1 and end = Math.min(skipCount +
pageSize, totalCount) and use the existing `${start}-${end} of ${totalCount}`
template.
- Around line 390-397: Wrap the JSON.parse(stdout) in a try/catch so non-JSON
responses (HTML/plaintext) are detected; on parse failure, inspect the raw
stdout string with isCaidoBroken(stdout) and call
invalidateAndKillCaido(context) if broken, then rethrow a descriptive error
including the raw stdout; keep the existing path that checks json.errors after
successful parsing (the symbols to change are JSON.parse(stdout), isCaidoBroken,
and invalidateAndKillCaido(context)).
- Around line 796-801: repeatRequest is currently calling viewRequest(context, {
requestId, part: "request" }) which returns paginated/formatted content and can
alter or truncate the original bytes; switch to retrieving the raw bytes instead
and replay those. Update the call in repeatRequest to request the raw data (e.g.
viewRequest(context, { requestId, part: "raw" }) or the equivalent raw-bytes
helper like getRequestRawBytes) and use that raw buffer/string for replay so
pagination/formatting does not corrupt the replayed request.
---
Duplicate comments:
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Around line 708-723: The current construction of the curl command by
interpolating headerArgs, bodyArg, method, url and proxyUrl into a single shell
string (variables headerArgs, bodyArg, cmd) is vulnerable to shell injection;
change this to build an argv array and call child_process.spawn/execFile (no
shell) so each header is a separate argument (e.g., push '-H', `${k}: ${v}` for
each header), add body via ['--data-raw', body] (or pipe body to stdin) instead
of embedding in a quoted string, and validate/strictly whitelist the HTTP method
and sanitize the URL and proxyUrl before using them; update the code that
creates cmd to produce an array of args and switch the invocation to
spawn/execFile to eliminate shell interpolation.
- Around line 549-559: The code currently constructs a RegExp from untrusted
input via new RegExp(pattern, "gim") (symbols: pattern, regex, regex.exec,
content, matches) which allows ReDoS; fix by validating or sanitizing the
incoming pattern before constructing a RegExp: run a safety check (e.g., using a
safe-regex/regexpu or equivalent heuristic) and reject or escape the pattern if
it is unsafe, falling back to a safe substring search (indexOf / includes) on
content; ensure regex construction is wrapped in a try/catch and only proceed
with regex.exec if the safety check passes to prevent catastrophic backtracking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c153725-9dc2-4a16-a35c-61c20a940d03
📒 Files selected for processing (5)
docker/Dockerfilee2b/template.tslib/ai/tools/utils/proxy-manager.tslib/ai/tools/utils/sandbox.tslib/system-prompt.ts
✅ Files skipped from review due to trivial changes (2)
- e2b/template.ts
- lib/ai/tools/utils/sandbox.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/system-prompt.ts
- proxy-manager.test.ts: ensureCaido lock, needs_start flow, --ui-domain flag, error handling, CAIDO_UI_URL env var - proxy-formatters.test.ts: list_requests table, send_request output, scope_rules formatting, empty states Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/components/tools/__tests__/proxy-formatters.test.ts (1)
105-137: Strengthen assertions for layout-sensitive output.These tests use
toContainheavily, which won’t catch spacing/alignment regressions in terminal-style tables. For at least one golden case per formatter, assert full multiline output (toBe) or snapshot the exact string.Also applies to: 165-179, 212-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/tools/__tests__/proxy-formatters.test.ts` around lines 105 - 137, The test uses many loose toContain assertions so layout/spacing regressions for the terminal table produced by formatListRequests can slip through; update this test (and the other formatter tests referenced in the comment) to include at least one "golden" assertion that checks the exact multiline output—either replace the main expectations with a full-string equality (expect(result).toBe(`...multi-line exact table...`)) or add a snapshot assertion (expect(result).toMatchInlineSnapshot(...) or toMatchSnapshot()) for formatListRequests and the other formatter functions tested around the indicated areas so alignment and spacing are verified exactly while keeping any remaining toContain checks for semantic bits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/tools/__tests__/proxy-formatters.test.ts`:
- Around line 9-91: The test duplicates production formatter logic (padRight,
formatListRequests, formatSendRequest, formatScopeRules) causing drift; remove
these local implementations and import the shared pure formatter functions from
the production module (e.g., export them from ProxyToolHandler.tsx or a new
helpers file) into app/components/tools/__tests__/proxy-formatters.test.ts so
the tests exercise the real formatters instead of reimplemented copies.
In `@lib/ai/tools/utils/__tests__/proxy-manager.test.ts`:
- Around line 203-216: Tests currently assert a string contains itself
(tautologies); instead invoke the public behavior that uses isCaidoBroken (e.g.,
call the ProxyManager instance's public request/send method which internally
calls sendRequest/isCaidoBroken — if sendRequest is private, access it via the
public API or via (proxyManager as any).sendRequest) with a fake HTML response
body containing "Could not acquire a connection to the database" and "Repository
operation failed", then assert the actual observable outcome (error thrown,
recovery invoked, state change, event emitted, or a specific log call) rather
than string self-containment; update the two it() cases to route inputs through
that public call and assert the real side-effect that indicates Caido
detection/recovery.
---
Nitpick comments:
In `@app/components/tools/__tests__/proxy-formatters.test.ts`:
- Around line 105-137: The test uses many loose toContain assertions so
layout/spacing regressions for the terminal table produced by formatListRequests
can slip through; update this test (and the other formatter tests referenced in
the comment) to include at least one "golden" assertion that checks the exact
multiline output—either replace the main expectations with a full-string
equality (expect(result).toBe(`...multi-line exact table...`)) or add a snapshot
assertion (expect(result).toMatchInlineSnapshot(...) or toMatchSnapshot()) for
formatListRequests and the other formatter functions tested around the indicated
areas so alignment and spacing are verified exactly while keeping any remaining
toContain checks for semantic bits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 659bba75-2396-4977-b603-25b1d36fe114
📒 Files selected for processing (2)
app/components/tools/__tests__/proxy-formatters.test.tslib/ai/tools/utils/__tests__/proxy-manager.test.ts
…gies
- Export formatProxyOutput from ProxyToolHandler.tsx
- Export isCaidoBroken from proxy-manager.ts
- Formatter tests now import real function instead of reimplementing
- isCaidoBroken tests call real function with positive/negative cases
(was: expect("foo").toContain("foo") tautology)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…map label - runGql: wrap JSON.parse in try/catch — detect broken Caido in HTML responses and trigger auto-restart instead of throwing parse error - repeatRequest: fetch raw base64 bytes directly via GraphQL instead of paginated/wrapped viewRequest output (was mangling request on replay) - listSitemap: fix "1-0 of 0" label when totalCount is 0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- sendRequest: encode URL, headers, body via base64 and decode inside shell to prevent command injection from malicious header values - sendRequest: sanitize HTTP method to uppercase alpha only - searchContent: reject regex patterns over 500 chars to mitigate ReDoS Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
lib/ai/tools/utils/proxy-manager.ts (1)
710-718:⚠️ Potential issue | 🟠 Major
repeatRequeststill rewrites the captured request on replay.Lines 710-718 and 738-740 force the body path through
body?: string+--data-raw, so Lines 832-854 have to decode the raw bytes as UTF-8 text, collapse line endings, andtrim()the body before replay. Lines 870-891 then keep derived headers likeContent-Length/Hosteven after the URL/body may have changed. Multipart uploads, binary payloads, and signed bodies will no longer round-trip faithfully.Please keep the captured body as bytes (
--data-binaryvia stdin/temp file is the usual fix) and let curl recalculate derived headers unless the caller explicitly overrides them.Also applies to: 738-740, 832-891
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools/utils/proxy-manager.ts` around lines 710 - 718, The replay logic in repeatRequest/sendRequest currently converts captured bodies to strings and forces --data-raw plus keeps derived headers (Content-Length/Host), breaking binary, multipart and signed payloads; preserve and pass the captured body as raw bytes (e.g., Buffer/Uint8Array) and send to curl via --data-binary from stdin or a temp file instead of --data-raw, avoid UTF-8 decoding/line-ending normalization in the repeatRequest flow, and ensure you do not re-add or preserve derived headers like Content-Length or Host unless the caller explicitly provided them in opts.headers; update sendRequest/repeatRequest to accept/use a raw byte body and only forward headers present in opts.headers.
🧹 Nitpick comments (3)
app/components/tools/ProxyToolHandler.tsx (2)
8-13: Consider typingpartprop more strictly.Using
anyfor thepartprop reduces type safety. If a shared interface for tool parts exists (or can be extracted from the AI SDK response type), consider using it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/tools/ProxyToolHandler.tsx` around lines 8 - 13, The prop "part" in the ProxyToolHandlerProps interface is typed as any; replace it with a stricter type by importing or declaring the proper shape used by the component (e.g., use the SDK type like AIResponsePart/ToolPart if available) and update ProxyToolHandlerProps to use that type for the part field; if no SDK type exists, extract a minimal local interface matching the fields accessed by ProxyToolHandler (e.g., id, content, metadata, type) and use that type in ProxyToolHandlerProps to restore type-safety.
160-163: Guard against undefinedr.showing.If
r.showingis missing from the payload, the header line will display "undefined". Consider adding a fallback.🔧 Suggested fix
const lines: string[] = [ - `${r.total_count} entr${r.total_count !== 1 ? "ies" : "y"} (${r.showing})`, + `${r.total_count} entr${r.total_count !== 1 ? "ies" : "y"}${r.showing ? ` (${r.showing})` : ""}`, "", ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/tools/ProxyToolHandler.tsx` around lines 160 - 163, The header construction in the lines array uses r.showing directly and can render "undefined"; update the template to provide a safe fallback (e.g., use nullish coalescing or a conditional) so the expression becomes something like `${r.total_count} entr${r.total_count !== 1 ? "ies" : "y"} (${r.showing ?? r.total_count ?? 0})`; modify the lines array definition in ProxyToolHandler (the const lines: string[] = [...]) to use this fallback for r.showing so missing payload fields don't display "undefined".app/components/tools/__tests__/proxy-formatters.test.ts (1)
221-263: Missing test coverage forview_sitemap_entry.The test suite covers
list_sitemapandunknown_toolfallback but skipsview_sitemap_entry, which has dedicated formatting logic informatViewSitemapEntry. Consider adding a test case to ensure that formatter path is exercised.📋 Suggested test case
describe("view_sitemap_entry", () => { it("should format entry with metadata and request", () => { const result = formatProxyOutput("view_sitemap_entry", { entry: { id: "1", label: "example.com", kind: "DOMAIN", metadata: { isTls: true, port: 443 }, request: { method: "GET", path: "/api", response: { status: 200 } }, }, }); expect(result).toContain("example.com DOMAIN (id:1)"); expect(result).toContain("https port 443"); expect(result).toContain("GET /api -> 200"); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/tools/__tests__/proxy-formatters.test.ts` around lines 221 - 263, Add a unit test exercising the "view_sitemap_entry" formatter path so formatViewSitemapEntry is covered: call formatProxyOutput("view_sitemap_entry", { entry: { id: "1", label: "example.com", kind: "DOMAIN", metadata: { isTls: true, port: 443 }, request: { method: "GET", path: "/api", response: { status: 200 } } } }) and assert the returned string contains the expected pieces (e.g., "example.com DOMAIN (id:1)", TLS/port text like "https port 443", and the request/response line "GET /api -> 200") to ensure formatProxyOutput delegates to formatViewSitemapEntry correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Around line 824-828: The current code only queries raw request data via runGql
and later infers TLS from a Host header containing ":443"; change the GraphQL
query (the runGql call that sets `data`) to also request the request's `isTls`,
`host`, and `port` fields from Caido, then replace the header-heuristic URL
construction with logic that builds the replay URL from those fields (use scheme
= isTls ? "https" : "http", host value, and include port when non-default)
before replaying; update any code that referenced the old Host-suffix check to
use these new fields instead.
- Around line 38-53: invalidateAndKillCaido currently leaves the critical
cleanup (killing caido-cli and removing CAIDO_TOKEN_FILE) in the async sandbox
command path so callers that fire-and-forget it (e.g., callers of
invalidateAndKillCaido from ensureCaido) can retry before the broken
process/token are gone; change invalidateAndKillCaido so the restart cleanup is
performed immediately before returning by (a) moving/removing any early return
points and ensuring caidoLock.delete stays at top, (b) executing the kill + rm
-f ${CAIDO_TOKEN_FILE} as the first guaranteed cleanup step (run and await the
sandbox.commands.run that does the pgrep/kill and rm, or use a synchronous
removal API if available) so the token is removed and process killed before
callers can proceed, and (c) update callers (ensureCaido and the other call
sites) to await invalidateAndKillCaido when they need the restart to be
completed; refer to invalidateAndKillCaido, ensureCaido, caidoLock,
CAIDO_TOKEN_FILE and sandbox.commands.run when making the changes.
- Around line 231-298: The lazy-start path that detects "needs_start" returns
before publishing the CAIDO_UI_URL; update the "needs_start" branch in
ensureCaido() to compute the UI URL (using the same logic that builds
uiDomainFlag / host + config.port) and publish/log CAIDO_UI_URL exactly as done
in the normal startup path (the same export/logging block currently at the end
of ensureCaido()), before the early return so the UI-sharing flow works when
Caido is started from scratch.
- Around line 1077-1092: Clamp the requested page to the valid range before
using it to compute skipCount, has_more, and the showing label: ensure page is
at least 1 and at most totalPages (computed from totalCount and pageSize) —
update the local page variable (used in skipCount, has_more, and the showing
string) to this clamped value so skipCount = (clampedPage - 1) * pageSize,
has_more uses clampedPage < totalPages, and the showing range uses clampedPage
when building `${skipCount + 1}-${Math.min(skipCount + pageSize, totalCount)} of
${totalCount}`; this mirrors paginateContent() behavior and keeps listSitemap()
outputs sane.
---
Duplicate comments:
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Around line 710-718: The replay logic in repeatRequest/sendRequest currently
converts captured bodies to strings and forces --data-raw plus keeps derived
headers (Content-Length/Host), breaking binary, multipart and signed payloads;
preserve and pass the captured body as raw bytes (e.g., Buffer/Uint8Array) and
send to curl via --data-binary from stdin or a temp file instead of --data-raw,
avoid UTF-8 decoding/line-ending normalization in the repeatRequest flow, and
ensure you do not re-add or preserve derived headers like Content-Length or Host
unless the caller explicitly provided them in opts.headers; update
sendRequest/repeatRequest to accept/use a raw byte body and only forward headers
present in opts.headers.
---
Nitpick comments:
In `@app/components/tools/__tests__/proxy-formatters.test.ts`:
- Around line 221-263: Add a unit test exercising the "view_sitemap_entry"
formatter path so formatViewSitemapEntry is covered: call
formatProxyOutput("view_sitemap_entry", { entry: { id: "1", label:
"example.com", kind: "DOMAIN", metadata: { isTls: true, port: 443 }, request: {
method: "GET", path: "/api", response: { status: 200 } } } }) and assert the
returned string contains the expected pieces (e.g., "example.com DOMAIN
(id:1)", TLS/port text like "https port 443", and the request/response line "GET
/api -> 200") to ensure formatProxyOutput delegates to formatViewSitemapEntry
correctly.
In `@app/components/tools/ProxyToolHandler.tsx`:
- Around line 8-13: The prop "part" in the ProxyToolHandlerProps interface is
typed as any; replace it with a stricter type by importing or declaring the
proper shape used by the component (e.g., use the SDK type like
AIResponsePart/ToolPart if available) and update ProxyToolHandlerProps to use
that type for the part field; if no SDK type exists, extract a minimal local
interface matching the fields accessed by ProxyToolHandler (e.g., id, content,
metadata, type) and use that type in ProxyToolHandlerProps to restore
type-safety.
- Around line 160-163: The header construction in the lines array uses r.showing
directly and can render "undefined"; update the template to provide a safe
fallback (e.g., use nullish coalescing or a conditional) so the expression
becomes something like `${r.total_count} entr${r.total_count !== 1 ? "ies" :
"y"} (${r.showing ?? r.total_count ?? 0})`; modify the lines array definition in
ProxyToolHandler (the const lines: string[] = [...]) to use this fallback for
r.showing so missing payload fields don't display "undefined".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2210b867-4093-43d9-aaa0-113b20f15d47
📒 Files selected for processing (4)
app/components/tools/ProxyToolHandler.tsxapp/components/tools/__tests__/proxy-formatters.test.tslib/ai/tools/utils/__tests__/proxy-manager.test.tslib/ai/tools/utils/proxy-manager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ai/tools/utils/tests/proxy-manager.test.ts
…isTls for replay, clamp sitemap page - Await invalidateAndKillCaido at all call sites to prevent race where retry hits the broken process before the kill completes - Extract exportCaidoUiUrl helper and call it on the needs_start path so first-time Caido launches publish CAIDO_UI_URL - Fetch isTls from Caido GraphQL instead of guessing TLS from :443 in Host header, which misses most HTTPS requests - Clamp out-of-range page in listSitemap to prevent nonsensical "showing" labels (matching paginateContent's existing pattern) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ai/tools/utils/proxy-manager.ts`:
- Around line 854-873: The header parsing treats header names as case-sensitive;
update the parsing in the block that builds reqHeaders (inside proxy-manager.ts
where reqHeaders: Record<string,string> is populated) to normalize header names
to a single case (e.g., store keys as lowercased:
reqHeaders[headerName.toLowerCase()]=...). Then change subsequent lookups that
expect "Host" or "Cookie" (the host = ... and cookie merge logic around lines
where host and cookie are read) to use the normalized keys ("host" and
"cookie"). Also scan the nearby code that references header keys (the logic
around host lookup and cookie merging) and adapt those references to the
lowercased key convention so header matching works regardless of original
casing.
- Around line 330-340: The fallback write to ~/.bashrc is not awaited, causing
exportCaidoUiUrl to return before persistence; update the code inside
exportCaidoUiUrl to await the fallback sandbox.commands.run(`echo 'export
CAIDO_UI_URL="${uiUrl}"' >> ~/.bashrc`, options) (preserving the existing catch
behavior) so the function only returns after the fallback completes; reference
sandbox.commands.run and exportCaidoUiUrl to locate where to add the await.
- Around line 852-871: The current parsing uses rawContent.split("\n") and
.trim(), which mutates original bytes and can break signed/multipart/binary
bodies; instead locate the header/body boundary directly on rawContent (search
for "\r\n\r\n" then fallback to "\n\n"), extract the header block and
request-line by slicing the raw string (not splitting/trimming the body), parse
headers from the header block lines (preserving header values except trimming
header names/values) to populate reqHeaders, and set reqBody to the raw
substring after the boundary (no .trim()). Update code paths that reference
rawContent, lines, reqMethod, urlPath, reqHeaders, bodyStart, reqBody, and host
to use this boundary-based slicing so the request body bytes remain unchanged
for replay.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab82a464-aa2b-416e-9741-243c9a42d712
📒 Files selected for processing (1)
lib/ai/tools/utils/proxy-manager.ts
HTTP header names are case-insensitive per RFC 7230, but the raw request parser stored them with original casing. Proxied traffic with lowercase headers (e.g. `host:`, `cookie:`) would fail with "No Host header" or miss cookie merges during replay. Normalize header keys to lowercase during parsing and update all lookups to match. Also fall back to the GraphQL `host` field when the raw Host header is absent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
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
Integrates Caido web security proxy into the agent, enabling automatic interception, inspection, and replay of all HTTP/HTTPS traffic during penetration testing.
list_requests,view_request,send_request,repeat_request,scope_rules,list_sitemap,view_sitemap_entryensureCaido(no always-on daemon)--ui-domain), localhost on Desktop. AI shares URL via$CAIDO_UI_URLenv var<proxy_interception>section for AI awarenessKey files
lib/ai/tools/utils/proxy-manager.tslib/ai/tools/proxy-tool.tslib/ai/tools/utils/caido-proxy.tsdocker/Dockerfilecaido-clibinaryapp/components/tools/ProxyToolHandler.tsxe2b/template.tslib/system-prompt.ts<proxy_interception>section,$CAIDO_UI_URLreferencelib/ai/tools/run-terminal-cmd.tsDeploy checklist
pnpm e2b:build:prodto rebuild production E2B template with caido-cliTest plan
tsc --noEmit)which caido-clireturns path, proxy tools work$CAIDO_UI_URL)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores