mirror of
https://github.com/usestrix/strix.git
synced 2026-10-03 02:24:24 +00:00
Accept stray characters in validation status and hide auto-fix guidance when it's off
This commit is contained in:
parent
8bd7f4cbd2
commit
818d583bd1
7 changed files with 165 additions and 18 deletions
|
|
@ -286,6 +286,37 @@ def _with_strictness(tool: FunctionTool, strict_schemas: bool) -> FunctionTool:
|
|||
return dataclasses.replace(tool, strict_json_schema=False)
|
||||
|
||||
|
||||
_VALIDATION_STATUS_ARG_DOC = re.compile(r"\n *validation_status:.*?(?=\n *\w+:|\Z)", re.DOTALL)
|
||||
_FIX_AGENTS_PARAGRAPH = re.compile(r"\n\nFix agents \(marked fix_task\).*?(?=\n\n)", re.DOTALL)
|
||||
|
||||
|
||||
def _without_auto_fix_guidance(tool: Tool) -> Tool:
|
||||
"""Hide what only applies when Fix agents run, for scans with auto-fix off.
|
||||
|
||||
Returns a copy so the shared tool singletons keep the guidance.
|
||||
"""
|
||||
if not isinstance(tool, FunctionTool):
|
||||
return tool
|
||||
if tool.name == finish_scan.name:
|
||||
return dataclasses.replace(
|
||||
tool, description=_FIX_AGENTS_PARAGRAPH.sub("", tool.description)
|
||||
)
|
||||
if tool.name not in {create_vulnerability_report.name, update_vulnerability_report.name}:
|
||||
return tool
|
||||
schema = tool.params_json_schema
|
||||
return dataclasses.replace(
|
||||
tool,
|
||||
description=_VALIDATION_STATUS_ARG_DOC.sub("", tool.description),
|
||||
params_json_schema={
|
||||
**schema,
|
||||
"properties": {
|
||||
k: v for k, v in schema["properties"].items() if k != "validation_status"
|
||||
},
|
||||
"required": [k for k in schema.get("required", []) if k != "validation_status"],
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def _function_tool_with_error_result(tool: FunctionTool) -> FunctionTool:
|
||||
invoke_tool = tool.on_invoke_tool
|
||||
|
||||
|
|
@ -673,6 +704,7 @@ def build_strix_agent(
|
|||
is_whitebox: bool = False,
|
||||
is_diff_scoped: bool = False,
|
||||
interactive: bool = False,
|
||||
auto_fix: bool = True,
|
||||
chat_completions_tools: bool = False,
|
||||
strict_tool_schemas: bool = True,
|
||||
system_prompt_context: dict[str, Any] | None = None,
|
||||
|
|
@ -683,6 +715,9 @@ def build_strix_agent(
|
|||
"""Build a SandboxAgent for either root or child use.
|
||||
|
||||
Args:
|
||||
auto_fix: Whether this scan starts Fix agents for confirmed findings.
|
||||
Off hides ``validation_status`` and Fix agent guidance from the
|
||||
tools and prompt.
|
||||
chat_completions_tools: Wrap SDK custom tools as function tools
|
||||
when the selected backend cannot accept Responses custom tools.
|
||||
strict_tool_schemas: Send function tools as strict-schema tools. Off
|
||||
|
|
@ -704,6 +739,7 @@ def build_strix_agent(
|
|||
is_root=is_root,
|
||||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
system_prompt_context=system_prompt_context,
|
||||
)
|
||||
|
||||
|
|
@ -717,6 +753,8 @@ def build_strix_agent(
|
|||
else:
|
||||
tools = [*selected_tools, *agent_tools, agent_finish]
|
||||
_ensure_unique_tool_names(tools)
|
||||
if not auto_fix:
|
||||
tools = [_without_auto_fix_guidance(tool) for tool in tools]
|
||||
tools = [
|
||||
_with_bounded_result(_with_strictness(_with_coerced_arguments(tool), strict_tool_schemas))
|
||||
if isinstance(tool, FunctionTool)
|
||||
|
|
@ -763,6 +801,7 @@ def make_child_factory(
|
|||
is_whitebox: bool = False,
|
||||
is_diff_scoped: bool = False,
|
||||
interactive: bool = False,
|
||||
auto_fix: bool = True,
|
||||
chat_completions_tools: bool = False,
|
||||
strict_tool_schemas: bool = True,
|
||||
system_prompt_context: dict[str, Any] | None = None,
|
||||
|
|
@ -783,6 +822,7 @@ def make_child_factory(
|
|||
is_whitebox=is_whitebox,
|
||||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
chat_completions_tools=chat_completions_tools,
|
||||
strict_tool_schemas=strict_tool_schemas,
|
||||
system_prompt_context=system_prompt_context,
|
||||
|
|
|
|||
|
|
@ -93,6 +93,7 @@ def render_system_prompt(
|
|||
is_root: bool = False,
|
||||
is_diff_scoped: bool = False,
|
||||
interactive: bool = False,
|
||||
auto_fix: bool = True,
|
||||
system_prompt_context: dict[str, Any] | None = None,
|
||||
include_scope: bool = True,
|
||||
) -> str:
|
||||
|
|
@ -143,6 +144,7 @@ def render_system_prompt(
|
|||
requested_skill_names=[name for name in skill_content if name not in shared],
|
||||
available_skills=get_available_skills(),
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
is_root=is_root,
|
||||
system_prompt_context=system_prompt_context or {},
|
||||
include_scope=include_scope,
|
||||
|
|
|
|||
|
|
@ -119,7 +119,7 @@ WHITE-BOX TESTING (code provided):
|
|||
- If dynamically running the code proves impossible after exhaustive attempts, pivot to comprehensive static analysis.
|
||||
- Try to infer how to run the code based on its structure and content.
|
||||
- Draft the initial fix candidate when you file the report. Use `code_locations` with verbatim `fix_before` and `fix_after`, plus `fix_pr_body`.
|
||||
- Treat the inline changes as a candidate, not as a completed fix. The automatically started Fix child can inspect and modify any required repository file.
|
||||
- Treat the inline changes as a candidate, not as a completed fix.{% if auto_fix %} The automatically started Fix child can inspect and modify any required repository file.{% endif %}
|
||||
- Record checks you ran in `fix_verification`. Do not describe reasoned checks as executed checks.
|
||||
|
||||
COMBINED MODE (code + deployed target present):
|
||||
|
|
@ -207,7 +207,7 @@ VALIDATION REQUIREMENTS:
|
|||
- Before filing any report, run the counterevidence pass: argue the strongest case AGAINST the finding, record what you found in the `counterevidence` field, set `confidence` honestly (a static-only trace you couldn't execute is at best `medium`), and state what evidence would change the severity. See the counterevidence and severity-calibration knowledge above.
|
||||
- A vulnerability is ONLY considered reported when a reporting agent uses create_vulnerability_report (or create_dependency_report for known-CVE dependency/supply-chain findings) with full details. Mentions in agent_finish, finish_scan, or generic messages are NOT sufficient
|
||||
- When source is available, the reporting agent files an initial fix candidate with the report. The candidate uses `code_locations` with `fix_before` and `fix_after`, plus `fix_pr_body`.
|
||||
- Do not treat the candidate as a prepared fix. After a confirmed source-backed report is saved, the runtime starts a Fix child for implementation and testing. Do not create a separate repair child.
|
||||
- Do not treat the candidate as a prepared fix.{% if auto_fix %} After a confirmed source-backed report is saved, the runtime starts a Fix child for implementation and testing.{% endif %} Do not create a separate repair child.
|
||||
- Do not silently patch a finding without filing a report.
|
||||
- DEDUPLICATION: The create_vulnerability_report tool uses LLM-based deduplication. If it rejects your report as a duplicate, DO NOT attempt to re-submit the same vulnerability. Accept the rejection and move on to testing other areas. The vulnerability has already been reported by another agent. If your evidence proves more than the finding it matched (a working exploit where that one had only a static trace, a chain that raises the impact), revise that finding with update_vulnerability_report using the duplicate_of id — never re-file it.
|
||||
- HTTP EVIDENCE: a finding you validated through the proxy is not fully filed until `http_exchange_ids` carries the proxy request ids of the exchanges that prove it — the request that triggers the vulnerability plus the baseline/control request it differs from (an unauthenticated success next to the authenticated one, the payload response next to the benign one). Copy the ids exactly as `list_requests`/`view_request` show them, never invent or guess one, and never omit the field to bypass validation. Leave it out only when there is no captured HTTP exchange at all (static-only code findings, dependency CVEs). If you filed before the proving exchanges existed, attach them afterwards with update_vulnerability_report. Without the ids, the finding ships as prose nobody can replay.
|
||||
|
|
@ -328,7 +328,7 @@ ROOT AGENT ROLE:
|
|||
|
||||
1. **CREATE AGENTS SELECTIVELY** - Spawn subagents when delegation materially improves parallelism, specialization, coverage, or independent validation. Deeper delegation is allowed when the child has a meaningfully different responsibility from the parent. Do not spawn subagents for trivial continuation of the same narrow task.
|
||||
2. **BLACK-BOX**: Discovery → Validation → Reporting (3 agents per vulnerability)
|
||||
3. **WHITE-BOX**: Discovery → Validation → Reporting with an initial fix candidate. Saving a confirmed source-backed report automatically starts a Fix child.
|
||||
3. **WHITE-BOX**: Discovery → Validation → Reporting with an initial fix candidate.{% if auto_fix %} Saving a confirmed source-backed report automatically starts a Fix child.{% endif %}
|
||||
4. **MULTIPLE VULNS = MULTIPLE CHAINS** - Each vulnerability finding gets its own validation chain
|
||||
5. **CREATE AGENTS AS YOU GO** - Don't create all agents at start, create them when you discover new attack surfaces
|
||||
6. **ONE JOB PER AGENT** - Each agent has ONE specific task only
|
||||
|
|
@ -371,27 +371,36 @@ Spawns "Auth Validation Agent" (proves it's exploitable)
|
|||
If valid → Spawns "Auth Reporting Agent" (creates the vulnerability report
|
||||
with the initial fix candidate: code_locations fix_before/fix_after
|
||||
+ fix_pr_body)
|
||||
{%- if auto_fix %}
|
||||
↓
|
||||
The runtime automatically starts a Fix child after the confirmed finding is saved.
|
||||
The Fix child implements and tests in its own worktree while assessment continues.
|
||||
{%- endif %}
|
||||
```
|
||||
|
||||
CONFIRMED FINDINGS AND FIXES:
|
||||
{%- if auto_fix %}
|
||||
- Set validation_status="confirmed" only after validation establishes the issue.
|
||||
Use "unconfirmed" for source concerns with unresolved evidence gaps.
|
||||
- Saving a confirmed, source-backed finding with an actionable candidate automatically
|
||||
starts a dedicated Fix child. Do not create repair children yourself or edit
|
||||
assessment source to fix vulnerabilities. Report findings and continue assessment.
|
||||
starts a dedicated Fix child.
|
||||
{%- endif %}
|
||||
- Do not create repair children yourself or edit assessment source to fix
|
||||
vulnerabilities. Report findings and continue assessment.
|
||||
{%- if auto_fix %}
|
||||
- The Fix child owns implementation, regression testing and customer unit tests in
|
||||
its own worktree. Candidate revisions automatically invalidate and replace old
|
||||
work. Fixes may continue after the security report is published.
|
||||
{%- endif %}
|
||||
- Keep assessment code unchanged for continued testing and attack chaining.
|
||||
- Other agents share this sandbox. Stop only your own processes using stop_process
|
||||
or Ctrl-C through your own write_stdin session. Never use process-wide cleanup
|
||||
such as pkill/killall or terminate a service because it occupies a port; choose
|
||||
another port instead.
|
||||
- Finish the assessment when security work is complete. Fix agents may still run;
|
||||
- Finish the assessment when security work is complete.
|
||||
{%- if auto_fix %} Fix agents may still run;
|
||||
finish_scan publishes the report and the runtime handles their eventual cleanup.
|
||||
{%- endif %}
|
||||
|
||||
CRITICAL RULES:
|
||||
|
||||
|
|
|
|||
|
|
@ -152,6 +152,7 @@ def _compose_root_instructions_override(
|
|||
is_whitebox: bool,
|
||||
is_diff_scoped: bool,
|
||||
interactive: bool,
|
||||
auto_fix: bool,
|
||||
system_prompt_context: dict[str, Any],
|
||||
) -> str | None:
|
||||
if root_instructions_override is None:
|
||||
|
|
@ -164,6 +165,7 @@ def _compose_root_instructions_override(
|
|||
is_root=True,
|
||||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
system_prompt_context=system_prompt_context,
|
||||
include_scope=False,
|
||||
)
|
||||
|
|
@ -465,6 +467,14 @@ async def run_strix_scan(
|
|||
except Exception:
|
||||
logger.exception("Failed to configure user MCP servers; continuing without them")
|
||||
|
||||
report_state = get_global_report_state()
|
||||
auto_fix = bool(
|
||||
scan_config.get("auto_fix_enabled", True) is not False
|
||||
and scan_config.get("mode") != "pr_review"
|
||||
and report_state is not None
|
||||
and local_sources
|
||||
)
|
||||
|
||||
root_context = _merge_root_prompt_context(scope_context, extra_system_prompt_context)
|
||||
root_instructions = _compose_root_instructions_override(
|
||||
root_instructions_override,
|
||||
|
|
@ -473,6 +483,7 @@ async def run_strix_scan(
|
|||
is_whitebox=is_whitebox,
|
||||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
system_prompt_context=root_context,
|
||||
)
|
||||
|
||||
|
|
@ -484,6 +495,7 @@ async def run_strix_scan(
|
|||
is_whitebox=is_whitebox,
|
||||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
chat_completions_tools=chat_completions_tools,
|
||||
strict_tool_schemas=strict_tool_schemas,
|
||||
system_prompt_context=root_context,
|
||||
|
|
@ -499,13 +511,7 @@ async def run_strix_scan(
|
|||
skills=skills,
|
||||
)
|
||||
|
||||
report_state = get_global_report_state()
|
||||
if (
|
||||
scan_config.get("auto_fix_enabled", True) is not False
|
||||
and scan_config.get("mode") != "pr_review"
|
||||
and report_state is not None
|
||||
and local_sources
|
||||
):
|
||||
if auto_fix and report_state is not None and local_sources:
|
||||
fixes = ScanFixes(
|
||||
session=sandbox_session,
|
||||
coordinator=coordinator,
|
||||
|
|
@ -524,6 +530,7 @@ async def run_strix_scan(
|
|||
is_whitebox=is_whitebox,
|
||||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
auto_fix=auto_fix,
|
||||
chat_completions_tools=chat_completions_tools,
|
||||
strict_tool_schemas=strict_tool_schemas,
|
||||
system_prompt_context=scope_context,
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import json
|
|||
import logging
|
||||
import re
|
||||
from pathlib import Path, PurePosixPath
|
||||
from typing import TYPE_CHECKING, Any, Literal, cast
|
||||
from typing import TYPE_CHECKING, Any, cast
|
||||
|
||||
from agents import RunContextWrapper, function_tool
|
||||
|
||||
|
|
@ -173,6 +173,7 @@ _REQUIRED_FIELDS = {
|
|||
|
||||
_VALID_FIX_EFFORT = frozenset({"trivial", "low", "medium", "high"})
|
||||
_VALID_CONFIDENCE = frozenset({"high", "medium", "low"})
|
||||
_VALID_VALIDATION_STATUS = frozenset({"confirmed", "unconfirmed"})
|
||||
_MAX_HTTP_EXCHANGE_IDS = 10
|
||||
_MAX_HTTP_EXCHANGE_ID_CHARS = 128
|
||||
|
||||
|
|
@ -552,6 +553,11 @@ _UPDATE_TEXT_FIELDS = (
|
|||
)
|
||||
|
||||
|
||||
def _normalize_validation_status(value: Any) -> str:
|
||||
"""Keep only letters, so stray quotes or spaces from the model still match."""
|
||||
return re.sub(r"[^a-z]", "", str(value).lower())
|
||||
|
||||
|
||||
def _collect_update_changes( # noqa: PLR0912, PLR0915
|
||||
fields: dict[str, Any],
|
||||
) -> tuple[dict[str, Any], list[str]]:
|
||||
|
|
@ -566,7 +572,8 @@ def _collect_update_changes( # noqa: PLR0912, PLR0915
|
|||
|
||||
validation_status = fields.get("validation_status")
|
||||
if validation_status is not None:
|
||||
if validation_status not in {"confirmed", "unconfirmed"}:
|
||||
validation_status = _normalize_validation_status(validation_status)
|
||||
if validation_status not in _VALID_VALIDATION_STATUS:
|
||||
errors.append("validation_status must be confirmed or unconfirmed")
|
||||
else:
|
||||
changes["validation_status"] = validation_status
|
||||
|
|
@ -986,7 +993,7 @@ async def _do_create( # noqa: PLR0911 - explicit validation and persistence out
|
|||
confidence_rationale: str | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
validation_status: Literal["confirmed", "unconfirmed"] = "unconfirmed",
|
||||
validation_status: str = "unconfirmed",
|
||||
fix_candidate_blocker: FixCandidateBlocker | None = None,
|
||||
agent_id: str | None = None,
|
||||
agent_name: str | None = None,
|
||||
|
|
@ -1022,6 +1029,10 @@ async def _do_create( # noqa: PLR0911 - explicit validation and persistence out
|
|||
f"Invalid fix_effort: {fix_effort!r}. Must be one of: {sorted(_VALID_FIX_EFFORT)}"
|
||||
)
|
||||
|
||||
validation_status = _normalize_validation_status(validation_status)
|
||||
if validation_status not in _VALID_VALIDATION_STATUS:
|
||||
errors.append("validation_status must be confirmed or unconfirmed")
|
||||
|
||||
errors.extend(_validate_cvss_breakdown(cvss_breakdown))
|
||||
|
||||
parsed_locations = _normalize_code_locations(code_locations)
|
||||
|
|
@ -1198,7 +1209,7 @@ async def create_vulnerability_report(
|
|||
confidence_rationale: str | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
validation_status: Literal["confirmed", "unconfirmed"] = "unconfirmed",
|
||||
validation_status: str = "unconfirmed",
|
||||
fix_candidate_blocker: FixCandidateBlocker | None = None,
|
||||
) -> str:
|
||||
"""File a vulnerability report — one report per fully-verified finding.
|
||||
|
|
@ -1665,7 +1676,7 @@ async def update_vulnerability_report(
|
|||
http_exchange_ids: list[str] | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
validation_status: Literal["confirmed", "unconfirmed"] | None = None,
|
||||
validation_status: str | None = None,
|
||||
fix_candidate_blocker: FixCandidateBlocker | None = None,
|
||||
contextual_cvss_reasoning: str | None = None,
|
||||
) -> str:
|
||||
|
|
|
|||
|
|
@ -128,3 +128,30 @@ def test_disabling_strict_leaves_shared_tools_untouched() -> None:
|
|||
agent = factory.build_strix_agent(is_root=True)
|
||||
|
||||
assert any(t.strict_json_schema for t in agent.tools if isinstance(t, FunctionTool))
|
||||
|
||||
|
||||
def _report_tool_props(agent: Any) -> list[dict[str, Any]]:
|
||||
return [
|
||||
t.params_json_schema["properties"]
|
||||
for t in agent.tools
|
||||
if isinstance(t, FunctionTool)
|
||||
and t.name in {"create_vulnerability_report", "update_vulnerability_report"}
|
||||
]
|
||||
|
||||
|
||||
def test_auto_fix_off_hides_fix_agent_guidance() -> None:
|
||||
"""Scans without Fix agents never see the field or guidance that assumes them."""
|
||||
off = factory.build_strix_agent(is_root=True, auto_fix=False)
|
||||
on = factory.build_strix_agent(is_root=True)
|
||||
|
||||
assert len(_report_tool_props(off)) == 2
|
||||
assert all("validation_status" not in props for props in _report_tool_props(off))
|
||||
assert all("validation_status" in props for props in _report_tool_props(on))
|
||||
|
||||
def agent_text(agent: Any) -> str:
|
||||
descriptions = [t.description for t in agent.tools if isinstance(t, FunctionTool)]
|
||||
return "\n".join([agent.instructions, *descriptions])
|
||||
|
||||
for phrase in ("validation_status", "Fix child", "Fix agent"):
|
||||
assert phrase not in agent_text(off)
|
||||
assert phrase in agent_text(on)
|
||||
|
|
|
|||
|
|
@ -1726,6 +1726,57 @@ async def test_evidence_only_update_reports_proxy_outage_as_retryable(
|
|||
assert report_state.vulnerability_reports[0] == original
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw_status", ['"confirmed"', " Confirmed. ", "'confirmed'"])
|
||||
async def test_update_accepts_validation_status_with_stray_characters(
|
||||
report_state: ReportState, raw_status: str
|
||||
) -> None:
|
||||
"""Some models wrap the value in quotes; that must not fail the call."""
|
||||
_seed_weak_report(report_state)
|
||||
ctx = ToolContext(
|
||||
context={"agent_id": "aaaa1111"},
|
||||
tool_name="update_vulnerability_report",
|
||||
tool_call_id="call-1",
|
||||
tool_arguments="{}",
|
||||
)
|
||||
raw = await update_vulnerability_report.on_invoke_tool(
|
||||
ctx,
|
||||
json.dumps(
|
||||
{
|
||||
"report_id": "vuln-0009",
|
||||
"update_reason": "The dynamic PoC proved the issue.",
|
||||
"validation_status": raw_status,
|
||||
}
|
||||
),
|
||||
)
|
||||
|
||||
assert json.loads(raw)["success"] is True
|
||||
assert report_state.vulnerability_reports[0]["validation_status"] == "confirmed"
|
||||
|
||||
|
||||
async def test_update_rejects_unknown_validation_status(report_state: ReportState) -> None:
|
||||
_seed_weak_report(report_state)
|
||||
ctx = ToolContext(
|
||||
context={"agent_id": "aaaa1111"},
|
||||
tool_name="update_vulnerability_report",
|
||||
tool_call_id="call-1",
|
||||
tool_arguments="{}",
|
||||
)
|
||||
raw = await update_vulnerability_report.on_invoke_tool(
|
||||
ctx,
|
||||
json.dumps(
|
||||
{
|
||||
"report_id": "vuln-0009",
|
||||
"update_reason": "Unsure.",
|
||||
"validation_status": "maybe",
|
||||
}
|
||||
),
|
||||
)
|
||||
|
||||
result = json.loads(raw)
|
||||
assert result["success"] is False
|
||||
assert "validation_status must be confirmed or unconfirmed" in json.dumps(result)
|
||||
|
||||
|
||||
def test_update_reports_persistence_failure_as_tool_error(report_state: ReportState) -> None:
|
||||
_seed_weak_report(report_state)
|
||||
original = dict(report_state.vulnerability_reports[0])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue