Seven targeted enhancements: recon enforcement, shared recon ledger, reproducible benchmarks, SDK compat guard, skill search, model override, request circuit-breaker - #1304
Open
khhvmc5g6f-eng wants to merge 16 commits into
Conversation
…model Two third-party benchmarks found Strix's exploitation subagents get spawned before real recon/threat-modeling happens, even though root_agent.md already tells the root agent (in prose) to recon-then-model first on black-box targets. An LLM can skip prose guidance under its own initiative, so add a code-level backstop: create_agent now attaches a non-blocking `warning` field when spawning a child with vulnerabilities-category skills before any threat model has been established for the scan, folding in a note when no reconnaissance coverage has been recorded either. The agent still spawns — this is a signal, not a gate. Also expose any_threat_model_exists() on the threat_model store and document the new warning in root_agent.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Agents (and the orchestrator picking create_agent(skills=[...])) had no way to find a skill by fuzzy need without already knowing its exact filename-derived name. Add a dependency-free search_skills(query, top_k) lexical scorer in strix/skills/__init__.py (name/description/body-preview token overlap, weighted, cached like existing metadata reads), expose it as a paired search_skills tool alongside load_skill, register it in the agent toolset, and point agents at it from the system prompt when the exact skill name isn't known. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a local, per-agent tracker that counts consecutive non-informative proxy responses (404s, or a non-2xx response shape repeating 3+ times in a row) and injects a strategy_warning field into repeat_request's result once a streak crosses 15, re-arming every further 15. This is distinct from the global $ budget and the per-turn tool-call limiter: it catches an agent burning budget one "reasonable" guessed-path request at a time, as reproduced by Protego's review (~1,300/~1,350 requests were 404s guessing conventional paths before ~$17 was spent with zero findings). Purely additive — never blocks or errors the call. Tracker state resets per scan alongside the other hydrate_* calls in runner.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Recon findings currently only live in prose (notes, threat model), so later agents re-guess at routes recon already resolved, wasting requests on 404s. Add strix/tools/recon_ledger, a coverage.py-style JSON-backed ledger (record_endpoint/update_endpoint/list_endpoints) that dedupes on (method, path) and rejects conflicting duplicates instead of overwriting them. Wire hydration into runner.py alongside hydrate_coverage_from_disk, register the tools in factory.py's _BASE_TOOLS, and point recon/root-agent skills at it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Strix's sandbox runtime (docker_client.py) and agent tool wiring (factory.py) reach into internal/undocumented openai-agents SDK shapes (a verbatim-copied private method, private helper functions, and a private session attribute) with no compile-time signal if a version bump changes them. Add strix/runtime/sdk_compat.py to declare the supported SDK range, assert it at startup (wired into strix.interface.main.main()), and document the concrete coupling checklist for future SDK bumps. Add docs/adr/0001 explaining the rationale and future direction, plus tests covering the version guard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add benchmarks/xben/: a JSON Schema for a single challenge run record, score.py (validates + aggregates committed run records into the results table, with a --check mode that fails loudly against benchmarks/README.md), run.py (drives real strix scans against XBEN challenges and emits conformant run records; needs Docker + an LLM key to execute for real), and a small set of clearly-marked synthetic fixture records under results/ so the harness can be exercised and unit tested without a live environment. Closes out the intent of usestrix#1263 and usestrix#1264: the headline "96% (100/104)" XBEN claim in benchmarks/README.md previously had no runnable evaluation code or committed data behind it in this repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Note that the hand-typed XBEN numbers are pending re-verification through benchmarks/xben/, and add a "Reproducing this benchmark" section pointing at its README. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add an optional model override to create_agent so the orchestrator can
deliberately spawn a "second opinion" specialist on a different LLM to
re-review surfaces the primary model already covered (e.g. needs_follow_up
coverage entries, or an under-covered target). The override threads
create_agent -> the spawner callback -> AgentCoordinator.register (persisted
for resume) -> build_strix_agent's SandboxAgent(model=...), falling back to
the run-wide model when omitted; ReportUsageHooks now records/estimates each
agent's cost against its own resolved model instead of always the run's
default. The model string is validated with a new shared
strix.config.models.invalid_model_reason() before spawning, returning a
clean {"success": False, "error": ...} on a bad string instead of a crash.
Also adds the scan_modes/second_opinion skill documenting when and how to
use this, and a short pointer to it in coordination/root_agent.md. No
automatic/heuristic triggering is added -- this is purely an explicit,
deliberately-invoked capability.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cements # Conflicts: # strix/core/runner.py
Contributor
|
| status = run_json.get("status") | ||
|
|
||
| matched = flag_was_matched(run_dir, challenge.flag) | ||
| solved = matched if challenge.flag else status == "completed" |
Contributor
There was a problem hiding this comment.
Ungraded Runs Count as Solved
When a challenge omits the optional flag, every scan that reaches the normal completed status is recorded as solved. That status only means the agent finished and generated a report; it does not mean the challenge was solved. score.py then counts the ungraded record as a success, so valid flag-less manifests can inflate the benchmark results.
Prompt To Fix With AI
This is a comment left during a code review.
Path: benchmarks/xben/run.py
Line: 243
Comment:
**Ungraded Runs Count as Solved**
When a challenge omits the optional `flag`, every scan that reaches the normal `completed` status is recorded as solved. That status only means the agent finished and generated a report; it does not mean the challenge was solved. `score.py` then counts the ungraded record as a success, so valid flag-less manifests can inflate the benchmark results.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+423
to
+424
| out_path = args.out_dir / f"{challenge.challenge_id}.{args.model.replace('/', '_')}.json" | ||
| out_path.write_text(json.dumps(record, indent=2) + "\n", encoding="utf-8") |
Contributor
There was a problem hiding this comment.
Challenge ID Escapes Output Directory
challenge_id comes from externally supplied challenge metadata and is inserted directly into the output filename. An identifier containing traversal segments can escape --out-dir, allowing the benchmark process to overwrite a matching JSON-suffixed file elsewhere on the host. The same unchecked identifier is also embedded in the Strix run name at line 348.
How this was verified: The manifest-controlled identifier reaches Path joining and write_text without separator validation or an output-directory containment check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: benchmarks/xben/run.py
Line: 423-424
Comment:
**Challenge ID Escapes Output Directory**
`challenge_id` comes from externally supplied challenge metadata and is inserted directly into the output filename. An identifier containing traversal segments can escape `--out-dir`, allowing the benchmark process to overwrite a matching JSON-suffixed file elsewhere on the host. The same unchecked identifier is also embedded in the Strix run name at line 348.
**How this was verified:** The manifest-controlled identifier reaches `Path` joining and `write_text` without separator validation or an output-directory containment check.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
477
to
+487
| } | ||
| if replay.get("error"): | ||
| payload["error"] = replay["error"] | ||
|
|
||
| if response is not None: | ||
| # Local, per-agent circuit breaker on unproductive requests (see | ||
| # unproductive_tracker.py) — distinct from the global $ budget and the | ||
| # per-turn tool-call-count limiter. Purely additive: never blocks or | ||
| # errors the call, just surfaces an actionable observation once an | ||
| # agent's own request stream shows a long run of dead ends. | ||
| warning = record_response( |
Contributor
There was a problem hiding this comment.
Request Tracker Misses Normal Traffic
The unproductive-request tracker observes only calls made through the host-side repeat_request tool. Requests sent through sandbox caido_api.repeat_request, Python HTTP clients, or fuzzers bypass record_response, even though the system prompt directs agents to use those paths for request-heavy testing. As a result, a long blind-fuzzing streak can continue without advancing the counter or producing a warning.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/tools/proxy/tools.py
Line: 477-487
Comment:
**Request Tracker Misses Normal Traffic**
The unproductive-request tracker observes only calls made through the host-side `repeat_request` tool. Requests sent through sandbox `caido_api.repeat_request`, Python HTTP clients, or fuzzers bypass `record_response`, even though the system prompt directs agents to use those paths for request-heavy testing. As a result, a long blind-fuzzing streak can continue without advancing the counter or producing a warning.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+145
to
+156
| for char in chunk: | ||
| if char.isdigit(): | ||
| digits += char | ||
| else: | ||
| break | ||
| if not digits: | ||
| break | ||
| parts.append(int(digits)) | ||
| if not parts: | ||
| msg = f"Could not parse a numeric version from {version!r}" | ||
| raise ValueError(msg) | ||
| return tuple(parts) |
Contributor
There was a problem hiding this comment.
Prerelease SDK Versions Pass Guard
The compatibility parser discards prerelease suffixes, so versions such as 0.19.0rc1 and 0.19.0.dev1 compare equal to stable 0.19.0 and pass the startup guard. This non-blocking hardening gap means a manually installed development or release-candidate SDK can reach code that depends on its private internals, despite not satisfying the declared package-version boundary.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/runtime/sdk_compat.py
Line: 145-156
Comment:
**Prerelease SDK Versions Pass Guard**
The compatibility parser discards prerelease suffixes, so versions such as `0.19.0rc1` and `0.19.0.dev1` compare equal to stable `0.19.0` and pass the startup guard. This non-blocking hardening gap means a manually installed development or release-candidate SDK can reach code that depends on its private internals, despite not satisfying the declared package-version boundary.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
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
1 participant
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
This PR is a set of 7 focused enhancements motivated by comparing Strix against other open-source AI pentesting agents (PentAGI, CAI/Cybersecurity-AI, hackingBuddyGPT) and against independent third-party benchmarks/reviews of Strix itself. Each change targets a concrete, evidenced gap rather than a hypothetical one — see the "Motivation" line under each item below.
All 7 are additive: no existing tool signatures had required fields changed, no default behavior changes without an explicit opt-in, and the full test suite passes (1848/1849 — the 1 failure,
test_resolves_common_bare_model_names, is a pre-existinglitellmmodel-catalog data-drift issue onmain, unrelated to this branch — confirmed by running it onmaindirectly).1. Warn before spawning exploitation agents without a threat model
Motivation:
root_agent.mdalready tells the orchestrator (in prose) to do recon and establish a threat model before spawning exploitation specialists in black-box mode, but this was pure prompt guidance with no code backstop. Independent reviews (Escape.tech grey-box benchmark: 1/20 known vulns found; a separate review: 1,300/1,350 requests wasted guessing wrong API paths) show this instruction gets skipped in practice.Change:
create_agentnow returns an additive"warning"field when spawning a child with exploitation-oriented skills before a threat model exists and no recon coverage has been recorded — a real, code-level signal the root agent can't silently ignore, without hard-blocking any legitimate flow.2. Shared recon endpoint ledger
Motivation: discovered routes/params/auth-state currently only live in an agent's own prose/context, so later agents re-guess at attack surface instead of building on recon findings — a direct contributor to the wasted-request problem above.
Change: new
record_endpoint/update_endpoint/list_endpointstools (strix/tools/recon_ledger/), architected identically to the existingcoverageledger (shared, persisted, deduped, hydrated on resume).3. Reproducible XBEN benchmark harness
Motivation:
benchmarks/README.md's "96% success rate" claim has no runnable evaluation code in this repo — only a link to an external repo. Open issues #1263 and #1264 already ask for exactly this: reproducibility from committed data and recorded provenance.Change:
benchmarks/xben/— a JSON Schema for run records, ascore.pythat aggregates committed results into the README table and has a--checkmode that fails if the README's claimed numbers don't match committed data, and arun.pyreproduction script against the real CLI surface. Committed example fixtures are clearly marked synthetic;score.py --checkcorrectly fails against them today, which is the intended proof the harness works — real historical run data still needs to be committed to back the existing headline number.4. Explicit
openai-agentsSDK compatibility boundaryMotivation:
docker_client.pysubclasses/patches private internals of theopenai-agentsSDK's sandbox implementation (not just its public API), pinned to a narrow version range with a comment noting upgrades need careful re-merging — currently that's tribal knowledge with no enforcement.Change:
strix/runtime/sdk_compat.pydocuments exactly which SDK internals are depended on (a concrete checklist) and addsassert_compatible_sdk_version(), called at startup, so an incompatible SDK version fails loudly and immediately with an actionable message instead of breaking silently mid-scan. Also fixed two stale version references indocker_client.py's docstrings. Plusdocs/adr/0001-openai-agents-sdk-coupling.md.5. Fuzzy skill search
Motivation: with ~70 markdown skills, an agent must already know the exact filename-derived skill name to use
load_skill/create_agent(skills=...)— there's no discovery path for "something about GraphQL introspection."Change:
search_skills(dependency-free lexical scoring over skill name/description/body) exposed as both a library function and a paired tool alongsideload_skill.6. Optional per-agent model override ("second opinion")
Motivation: every agent in a scan currently runs on the same single LLM. Published results from CAI (Cybersecurity AI) show composing different models on the same problem catches things a single model misses (19/33 vs 15/33 on Cybench). Strix had no mechanism to do this at all.
Change:
create_agent(..., model=...)lets the orchestrator deliberately spawn a specialist on an alternate model to re-review already-covered, uncertain surfaces (documented via new skillscan_modes/second_opinion.md). No automatic/heuristic triggering — purely an opt-in capability, so it can't unexpectedly double API costs. Also fixes a latent cost-reporting gap this surfaced:ReportUsageHooks.on_llm_endnow prices usage against the model the agent actually ran on rather than always the run-wide default.7. Circuit-breaker on unproductive request streaks
Motivation: the global
--max-budget-usdand per-turnTurnToolCallLimiterdon't catch an agent burning budget one "reasonable-looking" request at a time — exactly the reproduced failure mode of hundreds of consecutive 404s from guessed paths.Change: a per-agent, per-scan tracker on the proxy request tool that injects an additive
strategy_warningfield after 15 consecutive non-informative responses (404s, or a repeated non-2xx response shape), re-arming every 15 thereafter. Never blocks or errors the call.What this PR is not
It doesn't touch the global budget system, doesn't change any default model/behavior, and doesn't attempt a full SDK decoupling (unrealistic in one PR) or a heavyweight knowledge-graph memory system — each item was deliberately scoped to a real, evidenced gap rather than a larger rewrite.
Testing
uv run ruff check .— cleanuv run mypy strix/— clean, 134 source filesuv run pytest tests/ -q— 1848 passed, 1 pre-existing unrelated failure (confirmed present onmain)uv run strix --helpsmoke test passes after the SDK compat startup checkHappy to split this into separate PRs per item if that's preferred for review — flagging that tradeoff since this landed as one branch with 7 atomic commits.
🤖 Generated with Claude Code