From 007ed1a94e7dbf7b096c81e5b0354533ce94e0db Mon Sep 17 00:00:00 2001 From: alex s <46074070+bearsyankees@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:26:48 -0400 Subject: [PATCH] =?UTF-8?q?fix(reporting):=20restore=20create=5Fvulnerabil?= =?UTF-8?q?ity=5Freport=20parameter=20descrip=E2=80=A6=20(#1391)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(reporting): restore create_vulnerability_report parameter descriptions A docstring line beginning with a backtick fence example opened a markdown code block that griffe's Google-style parser never saw closed, so the Args section was parsed as plain text and the generated tool schema carried no per-parameter descriptions. Reword the example, move Args after the trailing notes so nothing after it is dropped from the tool description, and add a test asserting every scan-agent tool parameter has a description. * test(reporting): cover respond_to_user and reject null parameter descriptions --- strix/tools/reporting/tool.py | 110 +++++++++++++------------- tests/test_tool_param_descriptions.py | 45 +++++++++++ 2 files changed, 100 insertions(+), 55 deletions(-) create mode 100644 tests/test_tool_param_descriptions.py diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 45f4f658..e9820199 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -1075,11 +1075,11 @@ async def create_vulnerability_report( "Techniques" that read like an engineering runbook rather than a client deliverable. - **Use markdown in every text field**: ``**bold**`` for emphasis, - ``inline code`` for identifiers/values/parameters, and fenced - code blocks (```` ```language ````) for any code/payload/HTTP + ``inline code`` for identifiers/values/parameters, and + language-tagged fenced code blocks for any code/payload/HTTP excerpt. Never leave code bare/unformatted. When referencing a - file, annotate the fence, e.g. - ```` ```python title=app.py startLineNumber=42 endLineNumber=50 ````. + file, annotate the opening fence with + ``title=app.py startLineNumber=42 endLineNumber=50`` after the language. - Field discipline: ``poc_description`` is steps only — NO code (all code goes in ``poc_script_code``); ``remediation_steps`` is prose only — NO code/diffs (code fixes go in ``code_locations``). @@ -1191,6 +1191,57 @@ async def create_vulnerability_report( Broken / Risky Crypto, CWE-311 Missing Encryption, CWE-916 Weak Password Hashing. + Example (abbreviated — mirror this structure):: + + title: "Reflected XSS in /search q parameter" + description: + The **`q`** parameter of `/search` reflects user input into + the HTML response without encoding, allowing script + injection. + technical_analysis: + The handler interpolates `q` directly into the page body: + + ```python title=views.py startLineNumber=42 endLineNumber=44 + html = f"

Results for {q}

" + return HttpResponse(html) + ``` + + No output encoding is applied, so ` + ``` + evidence: + Response echoes the payload verbatim: + + ```html +

Results for

+ ``` + assumptions: + Assumes a victim can be induced to open a crafted link. + remediation_steps: + Context-encode all user input rendered into HTML; prefer the + template engine's auto-escaping over string interpolation. + counterevidence: + No output encoding, CSP, or WAF observed on this response; + payload executed in a current browser. The parameter is + reflected on an unauthenticated route, so no privileged + position is required. + confidence: "high" + severity_change_conditions: + A restrictive CSP that blocks inline script execution would + reduce impact and lower the severity. + fix_effort: "low" + + Nice to have: for code findings, if the checkout has git history, a quick + ``git blame`` (quote the paths) on the vulnerable line is worth weaving into + ``technical_analysis`` — who last touched it, when, and in which commit, as + part of the prose, not a separate section. Skip it if the line is + uncommitted or the command fails. + Args: title: Specific finding title (e.g. ``"SQL Injection in /api/users login parameter"``). Don't @@ -1359,57 +1410,6 @@ async def create_vulnerability_report( fix (summary + rationale). Prose/markdown only — the code change itself belongs in ``code_locations``. Omit for black-box findings. - - Example (abbreviated — mirror this structure):: - - title: "Reflected XSS in /search q parameter" - description: - The **`q`** parameter of `/search` reflects user input into - the HTML response without encoding, allowing script - injection. - technical_analysis: - The handler interpolates `q` directly into the page body: - - ```python title=views.py startLineNumber=42 endLineNumber=44 - html = f"

Results for {q}

" - return HttpResponse(html) - ``` - - No output encoding is applied, so ` - ``` - evidence: - Response echoes the payload verbatim: - - ```html -

Results for

- ``` - assumptions: - Assumes a victim can be induced to open a crafted link. - remediation_steps: - Context-encode all user input rendered into HTML; prefer the - template engine's auto-escaping over string interpolation. - counterevidence: - No output encoding, CSP, or WAF observed on this response; - payload executed in a current browser. The parameter is - reflected on an unauthenticated route, so no privileged - position is required. - confidence: "high" - severity_change_conditions: - A restrictive CSP that blocks inline script execution would - reduce impact and lower the severity. - fix_effort: "low" - - Nice to have: for code findings, if the checkout has git history, a quick - ``git blame`` (quote the paths) on the vulnerable line is worth weaving into - ``technical_analysis`` — who last touched it, when, and in which commit, as - part of the prose, not a separate section. Skip it if the line is - uncommitted or the command fails. """ ( http_exchange_ids, diff --git a/tests/test_tool_param_descriptions.py b/tests/test_tool_param_descriptions.py new file mode 100644 index 00000000..09150b44 --- /dev/null +++ b/tests/test_tool_param_descriptions.py @@ -0,0 +1,45 @@ +"""Every parameter of every scan-agent tool carries a description in its JSON schema. + +The model only sees a parameter's docstring text through the schema. An ``Args:`` +section that the docstring parser fails to recognise (for example because an +earlier line opens a markdown code fence it never closes) silently drops every +parameter description, so optional-but-conditionally-required fields such as +``confidence_rationale`` become anonymous nullable strings the model never fills. +""" + +from __future__ import annotations + +import pytest +from agents.tool import FunctionTool + +from strix.agents import factory +from strix.tools.agents_graph.tools import agent_finish +from strix.tools.finish.tool import finish_scan +from strix.tools.reporting.tool import create_vulnerability_report +from strix.tools.respond.tool import respond_to_user + + +_SCAN_AGENT_TOOLS = [ + tool + for tool in (*factory._BASE_TOOLS, finish_scan, agent_finish, respond_to_user) + if isinstance(tool, FunctionTool) +] + + +@pytest.mark.parametrize("tool", _SCAN_AGENT_TOOLS, ids=lambda tool: tool.name) +def test_every_parameter_has_a_description(tool: FunctionTool) -> None: + properties = tool.params_json_schema.get("properties", {}) + missing = sorted( + name + for name, schema in properties.items() + if not (isinstance(schema.get("description"), str) and schema["description"].strip()) + ) + assert not missing, f"{tool.name}: parameters without a description: {missing}" + + +def test_create_vulnerability_report_explains_confidence_rationale() -> None: + schema = create_vulnerability_report.params_json_schema["properties"] + assert "confidence" in create_vulnerability_report.params_json_schema["required"] + rationale = schema["confidence_rationale"]["description"] + assert "Required when" in rationale + assert "high" in rationale