-
Notifications
You must be signed in to change notification settings - Fork 51
Daily branch 2026 12 07 #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces several preinstalled pentesting tools in the Dockerfile, centralizes the preinstalled tools list, adds/export MAX_COMMAND_EXECUTION_TIME (10m), introduces an optional unified command timeout and removes PID/wait-specific handling in terminal command execution; tests updated to use the exported timeout constant. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant runTerminalCmd as run-terminal-cmd
participant Shell
participant SandboxMgr as hybrid-sandbox-manager
Caller->>runTerminalCmd: execute(command, is_background, timeout?)
runTerminalCmd->>SandboxMgr: use PREINSTALLED_PENTESTING_TOOLS (context)
runTerminalCmd->>Shell: spawn command (with effectiveStreamTimeout)
Shell-->>runTerminalCmd: stdout/stderr / exit
runTerminalCmd-->>Caller: stream output or timeout message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ai/tools/run-terminal-cmd.ts (1)
64-64: Update documentation to reflect the new 10-minute timeout.The guideline mentions "keep execution under 7 minutes", but the timeout has been increased to 10 minutes. Update this to maintain consistency with the new
MAX_TIMEOUT_SECONDSvalue.📝 Suggested documentation fix
-10. For pentesting tools, always use time-efficient flags and targeted scans to keep execution under 7 minutes (e.g., targeted ports for nmap, small wordlists for fuzzing, specific templates for nuclei, vulnerable-only enumeration for wpscan). Timeout handling: On timeout → reduce scope, break into smaller operations. +10. For pentesting tools, always use time-efficient flags and targeted scans to keep execution under 10 minutes (e.g., targeted ports for nmap, small wordlists for fuzzing, specific templates for nuclei, vulnerable-only enumeration for wpscan). Timeout handling: On timeout → reduce scope, break into smaller operations.
🤖 Fix all issues with AI agents
In @lib/ai/tools/utils/sandbox-command-options.ts:
- Line 4: The constant MAX_COMMAND_EXECUTION_TIME is not exported but tests
import it; update the declaration for MAX_COMMAND_EXECUTION_TIME in
sandbox-command-options.ts to export it (make it an exported constant) so
external modules/tests can import the value.
🧹 Nitpick comments (1)
lib/ai/tools/run-terminal-cmd.ts (1)
104-110: Consider simplifying the timeout handling.With
.default(DEFAULT_STREAM_TIMEOUT_SECONDS)in the Zod schema, thetimeoutparameter will never beundefinedat runtime (Zod applies the default before the execute function receives it). The null coalescing operator on line 127 is redundant but harmless.♻️ Optional simplification
You can simplify line 127 since Zod guarantees timeout is defined:
- const effectiveStreamTimeout = Math.min( - timeout ?? DEFAULT_STREAM_TIMEOUT_SECONDS, - MAX_TIMEOUT_SECONDS, - ); + const effectiveStreamTimeout = Math.min( + timeout, + MAX_TIMEOUT_SECONDS, + );Additionally, consider updating the TypeScript signature to reflect that timeout is always defined after Zod processing:
{ command, is_background, timeout, }: { command: string; is_background: boolean; - timeout?: number; + timeout: number; },Also applies to: 124-129
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docker/Dockerfilelib/ai/tools/__tests__/sandbox-capabilities.test.tslib/ai/tools/run-terminal-cmd.tslib/ai/tools/utils/sandbox-command-options.ts
💤 Files with no reviewable changes (1)
- docker/Dockerfile
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-28T01:16:26.536Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.536Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
Applied to files:
lib/ai/tools/utils/sandbox-command-options.tslib/ai/tools/__tests__/sandbox-capabilities.test.tslib/ai/tools/run-terminal-cmd.ts
🧬 Code graph analysis (1)
lib/ai/tools/run-terminal-cmd.ts (1)
lib/token-utils.ts (1)
TIMEOUT_MESSAGE(32-35)
🔇 Additional comments (3)
lib/ai/tools/__tests__/sandbox-capabilities.test.ts (1)
15-18: LGTM! Good refactoring to use the exported constant.Replacing hardcoded timeout values with the imported
MAX_COMMAND_EXECUTION_TIMEconstant improves maintainability and ensures tests stay in sync with the actual timeout value.Also applies to: 41-41, 51-51
lib/ai/tools/run-terminal-cmd.ts (2)
24-25: LGTM! Timeout constants are well-defined.The new constants provide clear default (60s) and maximum (600s) timeout values, aligning with the updated
MAX_COMMAND_EXECUTION_TIMEin sandbox configuration.
279-279: The timeout mechanism is already properly generalized and does not require changes.The code correctly uses
effectiveStreamTimeout(capped atMAX_TIMEOUT_SECONDS) for stream output timeout. TheonTimeouthandler does not kill processes—it only attempts PID discovery for user reference and returns current output while the process continues running with its ownMAX_COMMAND_EXECUTION_TIMEtimeout viacommonOptions. No specialized handling for specific command patterns (liketail --pidorwhile ps -p) exists in the codebase or was removed in this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lib/system-prompt.ts:
- Around line 20-37: The PREINSTALLED_PENTESTING_TOOLS constant currently lists
"vulnx/cvemap" but the Dockerfile only installs cvemap (not vulnx); update the
string inside PREINSTALLED_PENTESTING_TOOLS to remove "vulnx/" so it reads
"cvemap" (or just "cvemap (CVE vulnerability mapping)") to match actual
installed tooling; modify the exported constant definition
(PREINSTALLED_PENTESTING_TOOLS) accordingly so docs and runtime prompt align
with the Dockerfile.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docker/Dockerfilelib/ai/tools/utils/hybrid-sandbox-manager.tslib/system-prompt.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-28T01:16:26.536Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.536Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.ts
🧬 Code graph analysis (1)
lib/ai/tools/utils/hybrid-sandbox-manager.ts (1)
lib/system-prompt.ts (1)
PREINSTALLED_PENTESTING_TOOLS(21-37)
🔇 Additional comments (9)
lib/system-prompt.ts (1)
67-67: LGTM! Template substitution correctly centralizes the tools list.The replacement of the inline tools list with
${PREINSTALLED_PENTESTING_TOOLS}effectively centralizes the content and reduces duplication between the system prompt and the Docker sandbox environment.lib/ai/tools/utils/hybrid-sandbox-manager.ts (2)
7-7: LGTM! Import correctly added for the centralized tools list.The import of
PREINSTALLED_PENTESTING_TOOLSfrom the system prompt module is appropriate and aligns with the centralization effort across the codebase.
330-330: LGTM! Dynamic tool list injection for Docker sandbox.The replacement of the hardcoded tools list with
${PREINSTALLED_PENTESTING_TOOLS}successfully centralizes the toolset definition and ensures consistency between the system prompt and the Docker sandbox environment.docker/Dockerfile (6)
114-114: LGTM! Network discovery tool added.The addition of
arp-scanfor network discovery is appropriate and aligns with the updatedPREINSTALLED_PENTESTING_TOOLSlist in the system prompt.
138-139: LGTM! Web application security scanner added.The addition of
zaproxy(OWASP ZAP) is correctly placed and validated later in the tool validation section.
198-199: LGTM! Container vulnerability scanner added.The addition of
trivyfor container and filesystem vulnerability scanning is a valuable enhancement to the pentesting toolkit. The installation uses the official installation script from Aqua Security.
274-274: LGTM! Tool validation updated for new toolset.The validation checks have been correctly updated to include the newly added tools:
- Line 274:
arp-scan- Line 277:
katana- Line 278:
cvemap- Line 282:
trivy- Line 283:
zaproxyThis ensures the Docker image build will fail if any of these tools are not properly installed.
Also applies to: 277-278, 282-283
228-229: The claimed tool replacement cannot be verified in the current codebase.While katana (web crawler) and cvemap (CVE mapping) are correctly installed and documented in the system prompt, there are no references to dalfox or chisel anywhere in the codebase, making the claim about their removal and replacement unsubstantiated. The tools are properly integrated and functional, but the premise of the review comment is not supported by the current code state.
Likely an incorrect or invalid review comment.
190-190: No evidence of RSA CTF tool removal—this is the initial Dockerfile state.The Dockerfile was added as a new file in the repository with no git history of prior modifications. A comprehensive search found no references to any RSA CTF tool anywhere in the codebase. The line in question is part of the initial toolset, which includes jwt_tool, GitTools, trufflehog, trivy, and httpx. No removal occurred. If this comment is based on an AI-generated summary, please verify the actual changes before proceeding.
Likely an incorrect or invalid review comment.
| // Shared pentesting tools list for sandbox environments | ||
| export const PREINSTALLED_PENTESTING_TOOLS = `Pre-installed Pentesting Tools: | ||
| - Network Scanning: nmap (network mapping/port scanning), naabu (fast port scanner), httpx (HTTP prober) | ||
| - Subdomain/DNS: subfinder (subdomain enumeration), dnsrecon, dnsenum | ||
| - Web Fuzzing: ffuf (fast fuzzer), dirsearch (directory/file discovery), arjun (parameter discovery) | ||
| - Web Scanners: nikto (web server scanner), whatweb (web technology identifier), wpscan (WordPress scanner), wapiti (web vulnerability scanner), wafw00f (WAF detection) | ||
| - Injection: sqlmap (SQL injection detection/exploitation) | ||
| - Auth/Bruteforce: hydra (login bruteforcer) | ||
| - SMB/NetBIOS: smbclient, smbmap, nbtscan, python3-impacket, enum4linux | ||
| - Network Discovery: arp-scan | ||
| - Web Recon: gospider (web spider/crawler), katana (advanced web crawler) | ||
| - Git/Repository Analysis: gitdumper, gitextractor (dump/extract git repos) | ||
| - Secret Scanning: trufflehog (find credentials in git/filesystems) | ||
| - Vulnerability Assessment: nuclei (vulnerability scanner with templates), trivy (container/dependency scanner), zaproxy (OWASP ZAP), vulnx/cvemap (CVE vulnerability mapping) | ||
| - Forensics: binwalk, foremost (file carving) | ||
| - Utilities: gobuster, socat, proxychains4, hashid, libimage-exiftool-perl (exiftool), cewl | ||
| - Specialized: jwt_tool (JWT manipulation), interactsh-client (OOB interaction testing), SecLists (/home/user/SecLists or /usr/share/seclists) | ||
| - Documents: reportlab, python-docx, openpyxl, python-pptx, pandas, pypandoc, pandoc, odfpy`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the Dockerfile exists and examine its structure
find . -name "Dockerfile" -type f | head -5Repository: hackerai-tech/hackerai
Length of output: 85
🏁 Script executed:
# Look for the docker directory and list its contents
ls -la docker/ 2>/dev/null || echo "No docker directory found"Repository: hackerai-tech/hackerai
Length of output: 361
🏁 Script executed:
# If Dockerfile is elsewhere, search for it
fd -name "Dockerfile" -type fRepository: hackerai-tech/hackerai
Length of output: 296
🏁 Script executed:
# Read the Dockerfile to check tool installations
cat -n ./docker/Dockerfile | head -100Repository: hackerai-tech/hackerai
Length of output: 3609
🏁 Script executed:
# Check specifically for vulnx and cvemap installations
rg "vulnx|cvemap" ./docker/Dockerfile -A 2 -B 2Repository: hackerai-tech/hackerai
Length of output: 614
🏁 Script executed:
# Check for SecLists installation
rg "SecLists|seclists" ./docker/Dockerfile -A 2 -B 2Repository: hackerai-tech/hackerai
Length of output: 371
🏁 Script executed:
# Check for vulnx specifically in the entire Dockerfile
rg "vulnx" ./docker/DockerfileRepository: hackerai-tech/hackerai
Length of output: 48
🏁 Script executed:
# Get more context from the Dockerfile - read the rest of it
cat -n ./docker/Dockerfile | tail -100Repository: hackerai-tech/hackerai
Length of output: 5107
🏁 Script executed:
# Check the complete list of Go tools installed
rg "go install" ./docker/DockerfileRepository: hackerai-tech/hackerai
Length of output: 294
🏁 Script executed:
# Check for all Python packages installed via pip
rg "pip3 install|pip install" ./docker/Dockerfile -A 3Repository: hackerai-tech/hackerai
Length of output: 152
🏁 Script executed:
# Verify pypandoc and pandoc installations specifically
rg "pypandoc|pandoc" ./docker/DockerfileRepository: hackerai-tech/hackerai
Length of output: 93
Remove vulnx/ from the PREINSTALLED_PENTESTING_TOOLS list on line 33.
The Dockerfile installs only cvemap via Go (line 229), not vulnx. The documentation should list cvemap alone, not vulnx/cvemap. The SecLists path is correctly documented as /usr/share/seclists and matches the Dockerfile installation. All other tools in the list are properly installed and validated in the Dockerfile.
🤖 Prompt for AI Agents
In @lib/system-prompt.ts around lines 20 - 37, The PREINSTALLED_PENTESTING_TOOLS
constant currently lists "vulnx/cvemap" but the Dockerfile only installs cvemap
(not vulnx); update the string inside PREINSTALLED_PENTESTING_TOOLS to remove
"vulnx/" so it reads "cvemap" (or just "cvemap (CVE vulnerability mapping)") to
match actual installed tooling; modify the exported constant definition
(PREINSTALLED_PENTESTING_TOOLS) accordingly so docs and runtime prompt align
with the Dockerfile.
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.