mirror of
https://github.com/usestrix/strix.git
synced 2026-10-05 02:41:38 +00:00
fix(reporting): restore create_vulnerability_report parameter descrip… (#1391)
* 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
This commit is contained in:
parent
ef272b8e0d
commit
007ed1a94e
2 changed files with 100 additions and 55 deletions
|
|
@ -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"<h2>Results for {q}</h2>"
|
||||
return HttpResponse(html)
|
||||
```
|
||||
|
||||
No output encoding is applied, so `<script>` executes.
|
||||
poc_description:
|
||||
1. Navigate to `/search?q=<payload>`.
|
||||
2. Observe the payload executes in the victim's browser.
|
||||
poc_script_code:
|
||||
```
|
||||
GET /search?q=<script>alert(document.domain)</script>
|
||||
```
|
||||
evidence:
|
||||
Response echoes the payload verbatim:
|
||||
|
||||
```html
|
||||
<h2>Results for <script>alert(document.domain)</script></h2>
|
||||
```
|
||||
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"<h2>Results for {q}</h2>"
|
||||
return HttpResponse(html)
|
||||
```
|
||||
|
||||
No output encoding is applied, so `<script>` executes.
|
||||
poc_description:
|
||||
1. Navigate to `/search?q=<payload>`.
|
||||
2. Observe the payload executes in the victim's browser.
|
||||
poc_script_code:
|
||||
```
|
||||
GET /search?q=<script>alert(document.domain)</script>
|
||||
```
|
||||
evidence:
|
||||
Response echoes the payload verbatim:
|
||||
|
||||
```html
|
||||
<h2>Results for <script>alert(document.domain)</script></h2>
|
||||
```
|
||||
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,
|
||||
|
|
|
|||
45
tests/test_tool_param_descriptions.py
Normal file
45
tests/test_tool_param_descriptions.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Reference in a new issue