safety: fix the autonomous agent's unconfirmed shell-execution path + everything a deeper review found after #113 - #115
Merged
shuvonsec merged 15 commits intoAug 23, 2026
Conversation
_h() pointed at <repo-root>/hunt.py; the file is at tools/hunt.py. Even fixed, agent.py called _activate_recon_session/_resolve_recon_dir/ _resolve_findings_dir, none of which existed anywhere — the autonomous agent could not start a session at all. Implements all three, sanitizing domain against path traversal in the process. See SECURITY-REVIEW-2026-08-22.md finding #0.
agent.py's run_agent_hunt() derived session_dir by calling os.path.dirname(recon_dir), which stripped the session_id suffix and landed on the shared RECON_DIR/<domain>/sessions folder. All sessions for a domain then wrote to the same agent_session.json file, defeating session isolation entirely — --resume SESSION_ID could not restore that session's state since every session overwrote the same file. Fix: use recon_dir directly (already the correct full path); the bug was in the caller's incorrect derivation, not in _activate_recon_session(). Add two regression tests: - test_resume_specific_id_with_create_true: exact --resume combo - test_multiple_sessions_have_different_paths: critical regression test that verifies each session gets its own agent_session.json path
…M-proposed exploit command exploit_finding()/auto_triage_and_exploit() fed target-controlled response content into an LLM prompt, extracted a ```bash block from the reply, and ran it via shell=True with the FULL process environment (every configured provider API key) handed to the child — gated only by a 4-item denylist that doesn't block ';', '|', '$()', backticks, or reverse shells. A crafted target response could achieve autonomous RCE plus full credential exfiltration with zero human involvement. run_command() now requires typed confirmation at an interactive TTY before executing anything (same pattern as tools/spray_orchestrator.sh's hostname confirmation), and passes an explicit minimal environment instead of the full os.environ. The confirmation gate is unconditional whenever require_confirmation=True (the default) — no env var or flag can bypass it, and it refuses outright when stdin is not a TTY. ensure_tool()'s install-command call site now passes require_confirmation=False explicitly, since that command is looked up from the fixed _TOOL_INSTALL dict (never raw LLM output or target-controlled content) — the exploit_finding() call site is untouched and keeps the default (True), since that command IS LLM-derived. See SECURITY-REVIEW-2026-08-22.md finding Awarexone#1 (CRITICAL). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
…er + method policy into live dispatch Three gaps in the same "guard exists but isn't actually enforced on the live path" family: - dispatch() only ever checked the base --target domain against scope; subdomains/URLs discovered during recon were scanned with no further check. ScopeChecker.filter_file() now runs against recon output before any scanner reads it. - dispatch() hardcoded method='GET' for every tool, so SafeMethodPolicy's unsafe-method approval gate never fired even for state-changing tools (run_post_param_discovery, run_api_fuzz, run_sqlmap_*, etc). Added a real per-tool method map. - dispatch() never called guard.record_failure()/record_success(), so CircuitBreaker could never trip regardless of how many times a host errored. Now wired on both the exception and success paths. See SECURITY-REVIEW-2026-08-22.md findings Awarexone#2 (HIGH) and Awarexone#7 (MEDIUM). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
…est files Task 3's Step 5 only specified filtering recon-discovered URLs, missing the third fan-out vector named in finding Awarexone#2: run_sqlmap_on_file's actual target is data-driven from a request_file, not self.domain. A --target that's in scope says nothing about what host the request file itself targets — an operator could pass a scope-valid --target while a stale or malicious request_file points sqlmap at an entirely different host, and the only gate that fired was "REQUIRES APPROVAL" (a warning, not a block), giving a reviewing human no indication of the mismatch. Adds ToolDispatcher._parse_request_file_host(), a minimal line-scan for the request file's Host: header (not a full HTTP parser — sqlmap/Burp request files are well-formed raw requests). For run_sqlmap_on_file specifically: - an out-of-scope parsed host is now a hard BLOCKED, checked before the generic method-policy gate, so it can never merely "require approval" - an unparseable Host: header is treated as a scope-check failure and blocked, not assumed in-scope - an in-scope host still requires approval (POST), but the message now names the actual parsed target host so a human reviewing it can verify it matches intent See SECURITY-REVIEW-2026-08-22.md finding Awarexone#2 (HIGH). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
… LLM prompt No prompt builder in the codebase distinguished trusted instruction text from untrusted target-controlled content (recon output, scan findings, JS, response bodies) — everything was flat string concatenation. Adds tools.prompt_safety.delimit_untrusted(), which wraps untrusted content in labeled boundaries and neutralizes any text inside it that already looks like a boundary marker (closing a forged-delimiter escape). Applied everywhere target-derived content reaches a prompt in agent.py and brain.py, including HuntMemory.recent_observations() - the --resume path that reloads prior tool output into context (finding Awarexone#13). This doesn't make injection impossible, but it's the missing baseline defense, and it backstops Task 2's typed-confirmation gate: even if injected content convinces the model to propose something bad, the human confirmation step is what actually stops execution. See SECURITY-REVIEW-2026-08-22.md findings Awarexone#3 (HIGH) and Awarexone#13 (MEDIUM).
…task 4 follow-up) Task 4's original sweep covered the seven brain.py functions named in the brief's file list, but missed exploit_finding() and write_report() — both concatenate raw scanner-derived `evidence` straight into an LLM prompt via flat f-string interpolation, same as every other site this task fixed. The brief's file list omitted them, but the task's own rationale (finding Awarexone#3 backstops Finding Awarexone#1/Task 2's human-confirmation gate) applies most directly to exploit_finding(): injected evidence could still socially engineer the model into proposing a command that looks legitimate enough for a human to approve, even though execution itself is now gated. Wraps both with tools.prompt_safety.delimit_untrusted(), consistent with the labeling convention used elsewhere in this file. Adds regression tests (TestWriteReportEvidenceDelimited, TestExploitFindingEvidenceDelimited) that monkeypatch _stream / _stream_history to capture the constructed prompt and confirm a forged boundary marker embedded in evidence is neutralized rather than able to fake the real closing boundary. See SECURITY-REVIEW-2026-08-22.md finding Awarexone#3 (HIGH).
…ontrolled values engine.py's _run_shell and tools/hunt.py's run_graphql_audit both interpolated a target/URL into an f-string then ran it via shell=True. Double-quoting doesn't stop $(...)/backtick substitution inside a double-quoted shell argument. run_graphql_audit is the more serious of the two: its URLs come from gau/katana/wayback output — fully target-controlled, reachable via --graphql with no operator typo needed. Both now use argv-list Popen (shell=False). See SECURITY-REVIEW-2026-08-22.md findings Awarexone#4 and Awarexone#5 (HIGH).
Plain urllib.request.urlopen follows 3xx redirects with no destination check. A hostile in-scope target could 302 any of ~11 scanners into cloud metadata (169.254.169.254), localhost, or an RFC1918 address, using the operator's own machine as an SSRF proxy against their network. Adds tools/safe_http.py's safe_urlopen(), which validates each redirect hop's resolved hostname before following it, and swaps it in everywhere these scanners called urlopen directly (18 call sites across cors_scanner, crlf_scanner, nosqli_scanner, multipart_mutator, h1_idor_scanner, h1_race, h1_oauth_tester, waf_response_analyzer, learn, validate, eol_check). safe_urlopen forwards extra kwargs (e.g. context=) to the underlying opener on every hop, so the 4 call sites that pass a custom SSL context for scanning self-signed-cert targets keep that behavior unchanged. See SECURITY-REVIEW-2026-08-22.md finding Awarexone#8 (MEDIUM). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
Code review (verified against a live local HTTP server, not just the diff) found that _one_hop's opener never returns a redirect response at all: with _NoRedirectHandler.redirect_request() returning None, every handler in urllib's redirect chain declines, so it falls through to HTTPDefaultErrorHandler and raises HTTPError instead of returning the 3xx response. safe_urlopen's redirect-driving loop (status check, Location read, SSRF host check) therefore never ran against a real target — the previous tests only passed because they mocked _one_hop directly, bypassing urllib's actual redirect chain. Net effect: every one of the 18 swapped call sites broke on any real redirect (http-> https, www->apex, login flows), most silently mis-handling it as a generic HTTPError, and the SSRF guard itself had never executed for real. Fix: catch HTTPError in _one_hop and, for 3xx codes, return it as the hop's response (it satisfies .status/.headers same as a real response) - so the outer loop's validation logic actually runs. Also fixes a related correctness bug in the same reconstruction step: the next-hop request always dropped body/method, which is correct for 301/302/303 but wrong for 307/308 (which MUST preserve the original method and body per HTTP semantics). Now only 301/302/303 downgrade to a bodyless request; 307/308 carry the original method and data forward. Adds a non-mocked regression suite (TestSafeUrlopenRealServerRedirects) that spins up a real local HTTP server and drives safe_urlopen through the actual urllib opener/redirect chain: a same-host redirect followed to completion, a redirect to the cloud-metadata IP correctly blocked, and a 307 redirect that preserves POST method + body. Confirmed these fail against the pre-fix _one_hop (reproducing the exact HTTPError crash) and pass against the fix. See SECURITY-REVIEW-2026-08-22.md finding Awarexone#8 (MEDIUM), task 6 follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
Five files fell back to ssl.CERT_NONE + check_hostname=False whenever certifi couldn't be imported — and certifi isn't a declared dependency, so this was the DEFAULT state on any stock install, not an edge case. The OAuth password-grant spray tool sends credentials and receives access tokens over connections built this way. waf_response_analyzer.py disabled verification unconditionally, not just as a fallback. All now fall back to ssl.create_default_context() (system CA store, still verifying) instead of disabling verification outright. See SECURITY-REVIEW-2026-08-22.md finding Awarexone#9 (MEDIUM). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
…val injections, pin supply chain - AgentTracer no longer writes plaintext cookie/token/password values into agent_trace.jsonl or stdout — redacted before serialization. - engine.py's ~/.bughunter/config.json (stores provider API keys in plaintext) is now chmod 0600 after every write. - tools/cicd_scanner.sh and tools/h1_run.sh both ran eval "$CMD" built from an unsanitized positional arg / API-derived value — converted both to bash arrays, eval removed. - tools/zero_day_fuzzer.py's run_cmd used shell=True with an f-string URL; converted to argv-list Popen (shell=False). - install_tools.sh: pinned gau (v2.2.4), dalfox (v2.13.0 — v3.x is a Rust rewrite with no go.mod, not go-installable), subjack (pinned to the Go proxy's resolved pseudo-version, since subjack's own v2/v3 git tags are invalid Go modules and `go install .../subjack@v3.0.0` fails outright), and kerbrute (v1.0.3) go installs to specific verified versions instead of @latest. Verified sisaku-security/sisakulint is the correct, current canonical upstream (ultra-supara/sisakulint 301-redirects to the same repo ID — it's a rename/org-transfer, not a namesquat; ultra-supara is the original author's personal account, credited as such in README.md). Added checksum verification (curl -f + sha256sum -c against the published checksums.txt) to the sisakulint binary download. See SECURITY-REVIEW-2026-08-22.md findings Awarexone#6, Awarexone#10, Awarexone#11, Awarexone#12, Awarexone#15, and the cicd_scanner.sh/h1_run.sh LOW findings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
…cookie fix Reviewer flagged that tests/test_cookie_redaction.py only covered the trace-file redaction path, not the separate stdout-print path in ReActAgent.step() that the same task-8 fix addressed. Added test_cookie_arg_redacted_in_stdout_print, which exercises the exact AgentTracer.redact_args() + json.dumps() + print() pattern step() uses via capsys, asserting the cookie value never reaches stdout while "REDACTED" does. Verified this assertion fails if the stdout print is reverted to json.dumps(args) directly (i.e. it actually catches the regression it's meant to catch), not just that it currently passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
…ong-running tools near time budget exhaustion auto_triage_and_exploit() could trigger up to 25 * (1 triage + 6 exploit rounds) = 175 completions per run against whatever provider is configured, with no ceiling. Added max_completions (default 50), a true hard cap: it accounts for both triage calls (1 each) and exploit_finding calls (budgeted at its own internal 6-round cap), and skips exploit_finding rather than calling it if that would exceed the budget. Separately, ToolDispatcher now refuses to start any new network-facing tool once less than 10% of time_budget_hours remains (checked before the scope gate). Deviated from the brief's illustrative exemption for run_recon/run_vuln_scan(quick=True) as "short timeout" tools: tools/hunt.py shows run_recon has a 3600s subprocess timeout (the longest of any network tool, unaffected by quick) and run_vuln_scan is 1800s regardless of quick, so exempting them would have let the worst offender through. All NETWORK_TOOLS are gated uniformly instead. This doesn't preempt an in-flight subprocess (out of scope — would require changes to hunt.py's scanners), only reduces how many new ones can start this late. See SECURITY-REVIEW-2026-08-22.md findings Awarexone#14 and Awarexone#16. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
…recon URL files to scope Final-review fix wave, closing two findings against the completed round-2 hardening branch: 1. safe_urlopen(..., context=<ssl.SSLContext>) forwarded context= straight through _one_hop's **kwargs to OpenerDirector.open(), which (unlike module-level urlopen()) doesn't accept that kwarg — every call raised TypeError. tools/learn.py and tools/validate.py (including gate2_in_scope, a HackerOne scope gate) swallow it via a broad `except Exception` and silently no-op; waf_response_analyzer.py's _http_get doesn't catch TypeError and crashes outright. Fix: _one_hop now pops context out of kwargs and binds it to an HTTPSHandler on the opener instead of forwarding it to .open(). 2. ToolDispatcher._filter_recon_urls_to_scope only filtered urls/all.txt in place before each network-tool dispatch. tools/vuln_scanner.sh (run via run_vuln_scan, no approval gate) does active SQLi/XSS/SSTI probing off urls/with_params.txt, and also reads urls/js_files.txt, urls/api_endpoints.txt, and live/urls.txt — all derived from recon but never re-filtered, so out-of-scope hosts discovered via crawling/live-probing could still receive exploitation traffic. Fix: filter all five files (all.txt plus the four above) the same way, skipping any that don't exist yet. vuln_scanner.sh's priority-host files (critical/high/prioritized_hosts.txt) are currently never written by anything in this codebase, so filtering them would be dead code — left alone, noted for the record. Both fixes were TDD'd: a live-server regression test added to tests/test_safe_http_redirects.py exercises safe_urlopen(context=...) against a real opener.open() call end to end (confirmed TypeError before the fix, passes after); a new case in tests/test_agent_dispatcher_hardening.py seeds an out-of-scope URL into all five recon files and asserts all of them get cleaned on dispatch. Full suite: 698 -> 700 passed, no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qCJ5JVPgi38HLALD9N5wT
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
Follow-up to #113. That PR fixed scope enforcement in
AutopilotGuard, buta deeper review afterward found the fix had never actually been exercised
end-to-end — the autonomous agent couldn't even load its own dependencies
— plus a number of other real issues, including one genuinely critical
one. This PR closes all of it.
Headline: the autonomous agent couldn't run at all
agent.py's_h()looked forhunt.pyat the repo root; the file is attools/hunt.py. Every autonomous tool call failed withFileNotFoundErrorbefore ever reaching #113's scope gate. Even fixed,
agent.pycalled threesession-management functions (
_activate_recon_session,_resolve_recon_dir,_resolve_findings_dir) that didn't exist anywhere.Implemented all three, with path-traversal validation on
domain.Critical: unconfirmed autonomous shell execution in brain.py
exploit_finding()/auto_triage_and_exploit()embedded raw, target-controlled response content into an LLM prompt, extracted a ```bash block
from the reply, checked it against a 4-item denylist, and executed it via
shell=Truewith the full process environment (every configured LLMprovider API key) handed to the child — zero human confirmation. A crafted
target response could achieve autonomous RCE plus full credential
exfiltration. `run_command()` now requires typed confirmation at an
interactive TTY before executing anything derived from LLM output (same
pattern #113 already established for `spray_orchestrator.sh`), and passes
an explicit minimal environment instead of `os.environ`.
High-severity fixes
ToolDispatcherchecked--targetbut never the hosts scanners actually fan out to — recon-discovered URLs, or (the sharp edge) an arbitrary
Hostheader inside arun_sqlmap_on_filerequest file, completely unrelated to--target.Now scope-filters every recon-derived URL file before a scanner reads
it, and hard-blocks out-of-scope hosts smuggled via request files.
(recon output, scan findings, JS, response bodies — including the exact
evidencevariable driving the critical finding above) reached LLMprompts as flat string concatenation. Added
tools/prompt_safety. delimit_untrusted(), applied everywhere untrusted content enters aprompt, including the
--resumepath that reloads prior observations.hunt.py's GraphQL audit built ashell=Truestring fromgau/katana-harvested URLs (fully target-controlled);
engine.py's_run_shelldid the same with the CLItarget. Both converted to argv-list
Popen.no check on the destination — a hostile target could 302 into cloud
metadata or localhost. Added
tools/safe_http.safe_urlopen(), whichvalidates each redirect hop before following it.
sisakulintinstalled via sudo with no integrity check — addedchecksum verification and confirmed the publisher org is genuinely
canonical (not a namesquat — verified it's a GitHub org rename, same
underlying repo).
Medium-severity fixes
constructed but never actually enforced on the dispatch path — method
was hardcoded
GET, so the unsafe-method approval gate never fired evenfor state-mutating tools. All now wired for real.
CERT_NONEwhencertifi(nota declared dependency) was missing — the default install state, not an
edge case. Now falls back to
ssl.create_default_context()instead.redacted.
install_tools.sh'sgo install ...@latestcalls pinned to specificverified tags instead of floating
@latest.~/.bughunter/config.jsonnowchmod 0600.unbounded, up to ~175 completions/run against paid providers possible).
evalinjections (cicd_scanner.sh,h1_run.sh) convertedto arrays.
What this does NOT fix (disclosed, not silently left)
critical_hosts.txt/etc.) referenced byvuln_scanner.sharen't currently written by anything in thiscodebase — filtering them would be dead code, so they're untouched.
Flagging in case that changes.
hunt.py'srun_graphql_auditreadsurls/graphql.txtunfiltered —same bug class as the scope-filtering fix above, but confirmed not
reachable via the autonomous
ToolDispatcher(not inNETWORK_TOOLSor the dispatch chain) — appears to be a legacy/manual-CLI-only path.
spray_orchestrator.sh's shell-only path still never consultsScopeChecker— by design, relies on the human confirmation safety: wire scope enforcement into the live autopilot path (fail-closed); unbypassable spray confirmation #113already hardened, not a gap in this PR's scope.
Test plan
python3 -m pytest tests/ -q— 709 passed, 0 regressions againstcurrent
main(added ~75 new tests across the fixes above)failure scenario is now blocked — several verified against a real
local HTTP server (SSRF redirect handling) rather than mocks
ToolDispatcher.dispatch()control flowend to end to confirm no gate silently bypasses another
This was a large pass — happy to split into smaller PRs if that's easier
to review, just let me know how you'd like to receive it.