mirror of
https://github.com/usestrix/strix.git
synced 2026-11-14 09:01:25 +00:00
Merge 7eccdf8b9b into 851743adcf
This commit is contained in:
commit
146a7b9b3e
7 changed files with 480 additions and 19 deletions
|
|
@ -3,6 +3,7 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from agents.model_settings import ModelSettings
|
||||
|
|
@ -26,6 +27,18 @@ if TYPE_CHECKING:
|
|||
from strix.config.settings import ReasoningEffort
|
||||
|
||||
|
||||
def _mount_read_only() -> bool:
|
||||
"""Mirror of STRIX_MOUNT_READONLY in strix.interface.utils, for the prompt text."""
|
||||
return os.environ.get("STRIX_MOUNT_READONLY", "").strip().lower() in ("1", "true", "yes", "on")
|
||||
|
||||
|
||||
def _local_access(details: dict[str, Any]) -> str:
|
||||
"""How a local target is mounted, as the agent is told it."""
|
||||
if details.get("read_only") or _mount_read_only():
|
||||
return "mounted live and read-only: report on it, do not modify it"
|
||||
return "mounted live and writable — .git/.agents/.codex are read-only"
|
||||
|
||||
|
||||
def _accepts_required_tool_choice(model_name: str | None) -> bool:
|
||||
name = (model_name or "").strip().lower()
|
||||
for prefix in ("litellm/", "any-llm/"):
|
||||
|
|
@ -133,8 +146,7 @@ def build_root_task(scan_config: dict[str, Any]) -> str:
|
|||
path = details.get("target_path", "unknown")
|
||||
sections["Local Codebases"].append(
|
||||
f"- {path} (available at: {workspace_path}; "
|
||||
"this is the user's real directory, mounted live and writable — "
|
||||
".git/.agents/.codex are read-only)"
|
||||
f"this is the user's real directory, {_local_access(details)})"
|
||||
)
|
||||
elif ttype == "web_application":
|
||||
sections["URLs"].append(f"- {details.get('target_url', '')}")
|
||||
|
|
|
|||
|
|
@ -1310,8 +1310,19 @@ def is_whitebox_scan(targets_info: list[dict[str, Any]]) -> bool:
|
|||
return any(t.get("type") == "local_code" for t in targets_info or [])
|
||||
|
||||
|
||||
def _mount_read_only() -> bool:
|
||||
"""STRIX_MOUNT_READONLY: bind local targets into the sandbox read-only.
|
||||
|
||||
For review/localization scans the agent should inspect, not modify, the code
|
||||
under review — a read-only mount makes write/patch/delete physically impossible
|
||||
(prevents an agent from wiping or "fixing" the target instead of reporting on it).
|
||||
"""
|
||||
return os.environ.get("STRIX_MOUNT_READONLY", "").strip().lower() in ("1", "true", "yes", "on")
|
||||
|
||||
|
||||
def collect_local_sources(targets_info: list[dict[str, Any]]) -> list[dict[str, Any]]:
|
||||
local_sources: list[dict[str, Any]] = []
|
||||
force_ro = _mount_read_only()
|
||||
|
||||
for target_info in targets_info:
|
||||
details = target_info["details"]
|
||||
|
|
@ -1323,7 +1334,7 @@ def collect_local_sources(targets_info: list[dict[str, Any]]) -> list[dict[str,
|
|||
"source_path": details["target_path"],
|
||||
"workspace_subdir": workspace_subdir,
|
||||
"protect_metadata": True,
|
||||
"read_only": bool(details.get("read_only")),
|
||||
"read_only": bool(details.get("read_only")) or force_ro,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -1333,6 +1344,7 @@ def collect_local_sources(targets_info: list[dict[str, Any]]) -> list[dict[str,
|
|||
"source_path": details["cloned_repo_path"],
|
||||
"workspace_subdir": workspace_subdir,
|
||||
"protect_metadata": False,
|
||||
"read_only": force_ro,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from __future__ import annotations
|
|||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
from pathlib import PurePosixPath
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
|
@ -50,6 +51,48 @@ _CODE_LOCATION_FIELDS = (
|
|||
)
|
||||
|
||||
|
||||
# Sandbox mount root. Deliberately mirrored from strix.runtime.session_manager
|
||||
# rather than imported: that module pulls in the sandbox/Docker machinery
|
||||
# (agents.sandbox.*) at load, and this reporting tool is otherwise runtime-free.
|
||||
# The value is a fixed "/workspace"; keep the two in sync if it ever changes.
|
||||
# Agents see the target at /workspace/<subdir>/... and naturally report
|
||||
# code_locations with that absolute path; the report format wants it relative.
|
||||
_WORKSPACE_ROOT = "/workspace"
|
||||
|
||||
|
||||
def _to_workspace_relative(path: str) -> str:
|
||||
"""Make a sandbox-absolute code-location path repo-root-relative.
|
||||
|
||||
Agents report the absolute path they see in the sandbox (e.g.
|
||||
``/workspace/<subdir>/src/x.py``). Rejecting that as "must be relative" is the
|
||||
silent cause of findings-detected-but-never-filed (findings=N filed_reports=0):
|
||||
the report is dropped and the agent cannot file what it found. Strip the
|
||||
workspace root *and* the per-target mount subdir so the path is repo-root-relative
|
||||
(what SARIF and the report expect) and files. A path genuinely outside the
|
||||
workspace is left unchanged for the validator to reject.
|
||||
"""
|
||||
if not path:
|
||||
return path
|
||||
p = PurePosixPath(path)
|
||||
if not p.is_absolute():
|
||||
return path
|
||||
try:
|
||||
rel = p.relative_to(_WORKSPACE_ROOT)
|
||||
except ValueError:
|
||||
return path
|
||||
# The target is mounted at /workspace/<subdir>/..., where <subdir> is Strix's
|
||||
# per-target mount point, not part of the scanned repo. SARIF and the report
|
||||
# want a *repo-root-relative* path (strix/report/sarif.py: "File locations must
|
||||
# be repo-relative POSIX paths") so a consumer rooted at the repo — GitHub code
|
||||
# scanning, an IDE SARIF viewer — can resolve it; a leading <subdir>/ prefix
|
||||
# breaks that mapping. Drop the mount component too. A bare /workspace/<subdir>
|
||||
# (no file beneath it) has nothing to strip and is left as-is.
|
||||
parts = rel.parts
|
||||
if len(parts) > 1:
|
||||
return str(PurePosixPath(*parts[1:]))
|
||||
return str(rel)
|
||||
|
||||
|
||||
def _validate_file_path(path: str) -> str | None:
|
||||
if not path or not path.strip():
|
||||
return "file path cannot be empty"
|
||||
|
|
@ -84,6 +127,8 @@ def _normalize_code_locations(
|
|||
if field in ("snippet", "fix_before", "fix_after")
|
||||
else str(value).strip()
|
||||
)
|
||||
if field == "file":
|
||||
text = _to_workspace_relative(text)
|
||||
if text:
|
||||
normalized[field] = text
|
||||
if normalized.get("file") and normalized.get("start_line") is not None:
|
||||
|
|
@ -161,7 +206,12 @@ _REQUIRED_FIELDS = {
|
|||
"target": "Target cannot be empty",
|
||||
"technical_analysis": "Technical analysis cannot be empty",
|
||||
"poc_description": "PoC description cannot be empty",
|
||||
"poc_script_code": "PoC script/code is REQUIRED - provide the actual exploit/payload",
|
||||
"poc_script_code": (
|
||||
"PoC script/code is REQUIRED - provide the actual exploit/payload. "
|
||||
"If this is a real but non-exploitable weakness (hardcoded secret, missing "
|
||||
"hardening, contrived-only), or a downstream tool will triage/validate the "
|
||||
"findings, set STRIX_REQUIRE_POC=0 to file raw findings without a runnable PoC."
|
||||
),
|
||||
"remediation_steps": "Remediation steps cannot be empty",
|
||||
"evidence": "Evidence cannot be empty - provide concrete proof of the finding",
|
||||
"assumptions": "Assumptions cannot be empty - state exploitability prerequisites",
|
||||
|
|
@ -173,11 +223,79 @@ _MAX_HTTP_EXCHANGE_IDS = 10
|
|||
_MAX_HTTP_EXCHANGE_ID_CHARS = 128
|
||||
|
||||
|
||||
# A runnable PoC/exploit is mandatory by default (upstream's FP-reduction intent).
|
||||
# Setting STRIX_REQUIRE_POC to a false-y value turns it off for "raw findings" use —
|
||||
# SAST/SCA and defense-in-depth work, where many real, gradable weaknesses (hardcoded
|
||||
# secrets, missing hardening, contrived-only issues) have no practical exploit and
|
||||
# should still be filed and graded (down to informational), not dropped.
|
||||
_POC_OPTIONAL_FIELD = "poc_script_code"
|
||||
|
||||
|
||||
def _poc_required() -> bool:
|
||||
return os.environ.get("STRIX_REQUIRE_POC", "1").strip().lower() not in (
|
||||
"0",
|
||||
"false",
|
||||
"no",
|
||||
"off",
|
||||
)
|
||||
|
||||
|
||||
# Analysis fields that may be relaxed from required -> optional for weak-model / raw-findings
|
||||
# use. The finding still RECORDS: title/description/impact/target/code_locations/cvss/confidence
|
||||
# stay required. STRIX_RELAX_FINDING_FIELDS: "1"/"all" relaxes the whole set; a comma-list names
|
||||
# a subset (essentials can never be relaxed). Default empty = strict. An exploit-focused run keeps
|
||||
# them; a localization/triage run (findings fed to a downstream verifier) relaxes them so a
|
||||
# detected finding is recorded, not rejected for a missing field (usestrix/strix#1439).
|
||||
_RELAXABLE_FIELDS = frozenset(
|
||||
{
|
||||
"technical_analysis",
|
||||
"poc_description",
|
||||
"remediation_steps",
|
||||
"evidence",
|
||||
"assumptions",
|
||||
"counterevidence",
|
||||
"severity_change_conditions",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _relaxed_fields() -> frozenset[str]:
|
||||
raw = os.environ.get("STRIX_RELAX_FINDING_FIELDS", "").strip().lower()
|
||||
if not raw:
|
||||
return frozenset()
|
||||
if raw in ("1", "true", "yes", "on", "all"):
|
||||
return _RELAXABLE_FIELDS
|
||||
named = {f.strip() for f in raw.split(",") if f.strip()}
|
||||
unknown = named - _RELAXABLE_FIELDS
|
||||
if unknown:
|
||||
logger.warning(
|
||||
"STRIX_RELAX_FINDING_FIELDS: ignoring unrecognized field(s): %s (relaxable: %s)",
|
||||
", ".join(sorted(unknown)),
|
||||
", ".join(sorted(_RELAXABLE_FIELDS)),
|
||||
)
|
||||
return frozenset(named & _RELAXABLE_FIELDS)
|
||||
|
||||
|
||||
def _validate_required_text(fields: dict[str, str]) -> list[str]:
|
||||
"""Report every ``_REQUIRED_FIELDS`` entry that arrived blank."""
|
||||
return [
|
||||
msg for name, msg in _REQUIRED_FIELDS.items() if not str(fields.get(name) or "").strip()
|
||||
]
|
||||
"""Report every ``_REQUIRED_FIELDS`` entry that arrived blank.
|
||||
|
||||
``STRIX_REQUIRE_POC`` disabled allows a blank ``poc_script_code``;
|
||||
``STRIX_RELAX_FINDING_FIELDS`` additionally drops the named analysis fields, so a
|
||||
detected finding is still recorded rather than rejected for a missing analysis field.
|
||||
"""
|
||||
relaxed = _relaxed_fields()
|
||||
required = {k: v for k, v in _REQUIRED_FIELDS.items() if k not in relaxed}
|
||||
if not _poc_required():
|
||||
required = {k: v for k, v in required.items() if k != _POC_OPTIONAL_FIELD}
|
||||
missing = [msg for name, msg in required.items() if not str(fields.get(name) or "").strip()]
|
||||
skipped = sorted(
|
||||
n for n in relaxed if n in _REQUIRED_FIELDS and not str(fields.get(n) or "").strip()
|
||||
)
|
||||
if skipped:
|
||||
logger.info(
|
||||
"filing with relaxed-blank fields (STRIX_RELAX_FINDING_FIELDS): %s", ", ".join(skipped)
|
||||
)
|
||||
return missing
|
||||
|
||||
|
||||
def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]]:
|
||||
|
|
@ -309,13 +427,16 @@ def _validate_analysis_fields(
|
|||
) -> list[str]:
|
||||
"""Validate the counterevidence / confidence closure metadata."""
|
||||
errors: list[str] = []
|
||||
if not str(counterevidence or "").strip():
|
||||
relaxed = _relaxed_fields()
|
||||
ce = str(counterevidence or "").strip()
|
||||
scc = str(severity_change_conditions or "").strip()
|
||||
if "counterevidence" not in relaxed and not ce:
|
||||
errors.append(
|
||||
"Counterevidence cannot be empty - state the strongest evidence against "
|
||||
"this finding, or what you checked and found none (e.g. 'no input "
|
||||
"validation, WAF, or authorization check found on this path')"
|
||||
)
|
||||
if not str(severity_change_conditions or "").strip():
|
||||
if "severity_change_conditions" not in relaxed and not scc:
|
||||
errors.append(
|
||||
"severity_change_conditions cannot be empty - state the one concrete piece "
|
||||
"of evidence that would raise or lower the severity"
|
||||
|
|
@ -851,11 +972,23 @@ async def _do_create(
|
|||
errors.extend(http_exchange_errors)
|
||||
|
||||
if errors:
|
||||
# Log it: a rejected filing is otherwise invisible (returned to the agent, not
|
||||
# logged), so a finding detected-but-not-filed looks like silence. Surfacing the
|
||||
# rejected fields is how we see WHY a finding never lands (usestrix/strix#1439).
|
||||
logger.warning(
|
||||
"create_vulnerability_report rejected (title=%r): %d validation error(s): %s",
|
||||
(title or "")[:80],
|
||||
len(errors),
|
||||
"; ".join(errors[:8]),
|
||||
)
|
||||
return {"success": False, "error": "Validation failed", "errors": errors}
|
||||
|
||||
try:
|
||||
cvss_score, severity, _vector = _calculate_cvss(cvss_breakdown)
|
||||
except ValueError as exc:
|
||||
logger.warning(
|
||||
"create_vulnerability_report rejected (title=%r): CVSS: %s", (title or "")[:80], exc
|
||||
)
|
||||
return {"success": False, "error": "Validation failed", "errors": [str(exc)]}
|
||||
|
||||
try:
|
||||
|
|
@ -981,19 +1114,27 @@ def _caller_identity(ctx: RunContextWrapper) -> tuple[str | None, str | None]:
|
|||
@function_tool(timeout=180, strict_mode=False)
|
||||
async def create_vulnerability_report(
|
||||
ctx: RunContextWrapper,
|
||||
*,
|
||||
title: str,
|
||||
description: str,
|
||||
impact: str,
|
||||
target: str,
|
||||
technical_analysis: str,
|
||||
poc_description: str,
|
||||
poc_script_code: str,
|
||||
remediation_steps: str,
|
||||
evidence: str,
|
||||
assumptions: str,
|
||||
counterevidence: str,
|
||||
# Flag-gated fields default to "" so they are schema-OPTIONAL: STRIX_REQUIRE_POC
|
||||
# governs poc_script_code and STRIX_RELAX_FINDING_FIELDS the analysis fields. When a
|
||||
# flag is unset these are still required, but enforcement lives in the flag-aware
|
||||
# runtime validators (_validate_required_text / _validate_analysis_fields), not in a
|
||||
# static schema. A required-in-schema field would force the model to emit (and so
|
||||
# fabricate) a PoC even when the operator disabled the PoC requirement — see #1439.
|
||||
technical_analysis: str = "",
|
||||
poc_description: str = "",
|
||||
poc_script_code: str = "",
|
||||
remediation_steps: str = "",
|
||||
evidence: str = "",
|
||||
assumptions: str = "",
|
||||
counterevidence: str = "",
|
||||
severity_change_conditions: str = "",
|
||||
# Always required (never relaxable); no default keeps them schema-required.
|
||||
confidence: str,
|
||||
severity_change_conditions: str,
|
||||
fix_effort: str,
|
||||
cvss_breakdown: dict[str, str],
|
||||
endpoint: str | None = None,
|
||||
|
|
@ -1257,7 +1398,13 @@ async def create_vulnerability_report(
|
|||
target: Affected URL / domain / repository.
|
||||
technical_analysis: The mechanism and root cause.
|
||||
poc_description: Step-by-step reproduction (steps only, no code).
|
||||
poc_script_code: Working PoC (Python preferred).
|
||||
poc_script_code: Working PoC (Python preferred). Required by default;
|
||||
omit it only when the operator has set ``STRIX_REQUIRE_POC=0`` for a
|
||||
raw-findings run — never fabricate a PoC to satisfy the field.
|
||||
``technical_analysis``, ``poc_description``, ``remediation_steps``,
|
||||
``evidence``, ``assumptions``, ``counterevidence`` and
|
||||
``severity_change_conditions`` are likewise required unless the
|
||||
operator relaxed them via ``STRIX_RELAX_FINDING_FIELDS``.
|
||||
remediation_steps: Specific, actionable fix (prose, no code).
|
||||
evidence: Concrete proof the issue is real and exploitable —
|
||||
request/response excerpts, observed behavior, tool output.
|
||||
|
|
|
|||
63
tests/test_code_location_paths.py
Normal file
63
tests/test_code_location_paths.py
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
"""code_locations accept the sandbox-absolute path agents naturally report.
|
||||
|
||||
Agents see the target at /workspace/<subdir>/... and report code_locations with that
|
||||
absolute path. Rejecting it as "must be relative" silently drops the report — the
|
||||
cause of findings-detected-but-never-filed (findings=N filed_reports=0; see
|
||||
usestrix/strix#1439). The normalizer now strips the workspace root and the per-target
|
||||
mount subdir so the path is repo-root-relative and files, while paths genuinely outside
|
||||
the workspace are still rejected.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from strix.tools.reporting.tool import (
|
||||
_normalize_code_locations,
|
||||
_to_workspace_relative,
|
||||
_validate_code_locations,
|
||||
)
|
||||
|
||||
|
||||
def test_workspace_absolute_becomes_repo_relative() -> None:
|
||||
# The leading mount subdir (b8bLSpkh) is Strix's per-target mount point, not
|
||||
# part of the repo; SARIF/report consumers want a repo-root-relative path.
|
||||
assert (
|
||||
_to_workspace_relative("/workspace/b8bLSpkh/src/picklescan/scanner.py")
|
||||
== "src/picklescan/scanner.py"
|
||||
)
|
||||
|
||||
|
||||
def test_relative_path_unchanged() -> None:
|
||||
assert _to_workspace_relative("src/picklescan/scanner.py") == "src/picklescan/scanner.py"
|
||||
|
||||
|
||||
def test_outside_workspace_left_absolute() -> None:
|
||||
assert _to_workspace_relative("/etc/passwd") == "/etc/passwd"
|
||||
|
||||
|
||||
def test_bare_mount_subdir_has_nothing_to_strip() -> None:
|
||||
# /workspace/<subdir> with no file beneath it: only the subdir component is
|
||||
# left after stripping the root, and there is nothing repo-relative to keep.
|
||||
assert _to_workspace_relative("/workspace/b8bLSpkh") == "b8bLSpkh"
|
||||
|
||||
|
||||
def test_absolute_workspace_location_normalizes_and_validates() -> None:
|
||||
loc = [
|
||||
{
|
||||
"file": "/workspace/b8bLSpkh/src/picklescan/scanner.py",
|
||||
"start_line": 10,
|
||||
"end_line": 12,
|
||||
"snippet": "x",
|
||||
}
|
||||
]
|
||||
normalized = _normalize_code_locations(loc)
|
||||
assert normalized is not None
|
||||
assert normalized[0]["file"] == "src/picklescan/scanner.py"
|
||||
assert _validate_code_locations(normalized) == [] # files cleanly now
|
||||
|
||||
|
||||
def test_outside_workspace_absolute_still_rejected() -> None:
|
||||
loc = [{"file": "/etc/passwd", "start_line": 1, "end_line": 1}]
|
||||
normalized = _normalize_code_locations(loc)
|
||||
assert normalized is not None
|
||||
errs = _validate_code_locations(normalized)
|
||||
assert errs and "must be relative" in errs[0]
|
||||
50
tests/test_mount_readonly.py
Normal file
50
tests/test_mount_readonly.py
Normal file
|
|
@ -0,0 +1,50 @@
|
|||
"""STRIX_MOUNT_READONLY binds local targets read-only.
|
||||
|
||||
For review/localization scans the agent must inspect, not modify, the code under
|
||||
review. A read-only mount makes write/patch/delete physically impossible, so an
|
||||
agent can't wipe or "fix" the target instead of reporting on it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from strix.interface.utils import collect_local_sources
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
import pytest
|
||||
|
||||
|
||||
def _local_code() -> list[dict]:
|
||||
return [{"type": "local_code", "details": {"target_path": "/work/x", "workspace_subdir": "x"}}]
|
||||
|
||||
|
||||
def _repo() -> list[dict]:
|
||||
details = {"cloned_repo_path": "/work/r", "workspace_subdir": "r"}
|
||||
return [{"type": "repository", "details": details}]
|
||||
|
||||
|
||||
def test_default_is_writable(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("STRIX_MOUNT_READONLY", raising=False)
|
||||
assert collect_local_sources(_local_code())[0]["read_only"] is False
|
||||
assert collect_local_sources(_repo())[0]["read_only"] is False
|
||||
|
||||
|
||||
def test_env_forces_read_only(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
for val in ("1", "true", "yes", "on"):
|
||||
monkeypatch.setenv("STRIX_MOUNT_READONLY", val)
|
||||
assert collect_local_sources(_local_code())[0]["read_only"] is True, val
|
||||
assert collect_local_sources(_repo())[0]["read_only"] is True, val
|
||||
|
||||
|
||||
def test_off_values_stay_writable(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
for val in ("", "0", "false", "no"):
|
||||
monkeypatch.setenv("STRIX_MOUNT_READONLY", val)
|
||||
assert collect_local_sources(_local_code())[0]["read_only"] is False, val
|
||||
|
||||
|
||||
def test_per_target_read_only_still_honored(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("STRIX_MOUNT_READONLY", raising=False)
|
||||
ti = [{"type": "local_code", "details": {"target_path": "/work/x", "read_only": True}}]
|
||||
assert collect_local_sources(ti)[0]["read_only"] is True
|
||||
97
tests/test_relax_finding_fields.py
Normal file
97
tests/test_relax_finding_fields.py
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
"""STRIX_RELAX_FINDING_FIELDS relaxes a subset of create_vulnerability_report's
|
||||
required analysis fields so a weak model / raw-findings run can still RECORD a
|
||||
finding (essentials stay required) instead of having it rejected for a missing
|
||||
analysis field — the create-side cause of findings-detected-but-never-filed
|
||||
(usestrix/strix#1439). Deterministic: all validators are pure, no LLM.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from strix.tools.reporting.tool import (
|
||||
_relaxed_fields,
|
||||
_validate_analysis_fields,
|
||||
_validate_required_text,
|
||||
)
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
import pytest
|
||||
|
||||
_MINIMAL = {
|
||||
"title": "SSTI in /hello",
|
||||
"description": "user input into render_template_string",
|
||||
"impact": "RCE",
|
||||
"target": "/workspace/app",
|
||||
"technical_analysis": "",
|
||||
"poc_description": "",
|
||||
"poc_script_code": "",
|
||||
"remediation_steps": "",
|
||||
"evidence": "",
|
||||
"assumptions": "",
|
||||
}
|
||||
|
||||
|
||||
def _analysis_errs() -> list[str]:
|
||||
return _validate_analysis_fields(
|
||||
counterevidence="",
|
||||
confidence="high",
|
||||
confidence_rationale=None,
|
||||
severity_change_conditions="",
|
||||
)
|
||||
|
||||
|
||||
def test_relaxed_fields_parsing(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("STRIX_RELAX_FINDING_FIELDS", raising=False)
|
||||
assert _relaxed_fields() == frozenset()
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "all")
|
||||
assert "counterevidence" in _relaxed_fields() and "evidence" in _relaxed_fields()
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "counterevidence,assumptions")
|
||||
assert _relaxed_fields() == frozenset({"counterevidence", "assumptions"})
|
||||
# essentials can never be relaxed (not in the relaxable set)
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "title,description,evidence")
|
||||
assert _relaxed_fields() == frozenset({"evidence"})
|
||||
|
||||
|
||||
def test_relaxed_fields_warns_on_unknown(
|
||||
monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
# A typo'd field name must not be silently ignored.
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "evidence,technical_analsis")
|
||||
with caplog.at_level(logging.WARNING):
|
||||
assert _relaxed_fields() == frozenset({"evidence"})
|
||||
assert "technical_analsis" in caplog.text
|
||||
assert "unrecognized" in caplog.text.lower()
|
||||
|
||||
|
||||
def test_strict_rejects_blank_analysis_fields(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("STRIX_RELAX_FINDING_FIELDS", raising=False)
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", "0")
|
||||
assert _validate_required_text(_MINIMAL) # technical_analysis/evidence/... still required
|
||||
assert _analysis_errs() # counterevidence + severity_change_conditions required
|
||||
|
||||
|
||||
def test_relax_all_files_with_essentials_only(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", "0")
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "all")
|
||||
assert _validate_required_text(_MINIMAL) == [] # essentials present, analysis fields relaxed
|
||||
assert _analysis_errs() == []
|
||||
|
||||
|
||||
def test_relax_subset_only_drops_named(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", "0")
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "counterevidence,severity_change_conditions")
|
||||
# the two analysis-validator fields are relaxed...
|
||||
assert _analysis_errs() == []
|
||||
# ...but the text analysis fields (evidence/assumptions/...) are still required
|
||||
assert _validate_required_text(_MINIMAL)
|
||||
|
||||
|
||||
def test_essentials_still_required_when_blank(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", "0")
|
||||
monkeypatch.setenv("STRIX_RELAX_FINDING_FIELDS", "all")
|
||||
blank_title = {**_MINIMAL, "title": ""}
|
||||
errs = _validate_required_text(blank_title)
|
||||
assert any("Title" in e for e in errs) # essential not relaxable
|
||||
80
tests/test_reporting_poc_optional.py
Normal file
80
tests/test_reporting_poc_optional.py
Normal file
|
|
@ -0,0 +1,80 @@
|
|||
"""STRIX_REQUIRE_POC flag: PoC mandatory by default, optional for raw-findings use."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from strix.tools.reporting import tool as reporting_tool
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
import pytest
|
||||
|
||||
|
||||
def _all_filled_but_poc() -> dict[str, str]:
|
||||
fields = dict.fromkeys(reporting_tool._REQUIRED_FIELDS, "x")
|
||||
fields["poc_script_code"] = "" # no runnable exploit
|
||||
return fields
|
||||
|
||||
|
||||
def test_poc_required_by_default(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("STRIX_REQUIRE_POC", raising=False)
|
||||
assert reporting_tool._poc_required() is True
|
||||
errors = reporting_tool._validate_required_text(_all_filled_but_poc())
|
||||
assert any("PoC" in e for e in errors) # blank PoC rejected
|
||||
|
||||
|
||||
def test_poc_optional_when_flag_off(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", "0")
|
||||
assert reporting_tool._poc_required() is False
|
||||
# blank PoC + everything else present -> no errors (finding is filable)
|
||||
assert reporting_tool._validate_required_text(_all_filled_but_poc()) == []
|
||||
|
||||
|
||||
def test_flag_off_still_enforces_other_fields(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", "off")
|
||||
fields = _all_filled_but_poc()
|
||||
fields["evidence"] = "" # a non-PoC required field left blank
|
||||
errors = reporting_tool._validate_required_text(fields)
|
||||
assert any("Evidence" in e for e in errors)
|
||||
assert not any("PoC" in e for e in errors) # PoC still exempt
|
||||
|
||||
|
||||
def test_flag_falsey_and_truthy_values(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
for off in ("0", "false", "no", "off", "False", "OFF"):
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", off)
|
||||
assert reporting_tool._poc_required() is False, off
|
||||
for on in ("1", "true", "yes", ""): # empty/ambiguous -> safe default (required)
|
||||
monkeypatch.setenv("STRIX_REQUIRE_POC", on)
|
||||
assert reporting_tool._poc_required() is True, on
|
||||
|
||||
|
||||
# The runtime validators above are the single source of required-ness, because the
|
||||
# flags are per-run env and the tool schema is built once at import. The schema must
|
||||
# therefore mark the flag-gated fields OPTIONAL — otherwise a required-in-schema PoC
|
||||
# forces the model to emit (and fabricate) one even when STRIX_REQUIRE_POC=0. The
|
||||
# always-required essentials stay schema-required.
|
||||
_SCHEMA_OPTIONAL = frozenset(
|
||||
{
|
||||
"poc_script_code",
|
||||
"technical_analysis",
|
||||
"poc_description",
|
||||
"remediation_steps",
|
||||
"evidence",
|
||||
"assumptions",
|
||||
"counterevidence",
|
||||
"severity_change_conditions",
|
||||
}
|
||||
)
|
||||
_SCHEMA_REQUIRED = frozenset(
|
||||
{"title", "description", "impact", "target", "confidence", "fix_effort", "cvss_breakdown"}
|
||||
)
|
||||
|
||||
|
||||
def test_flag_gated_fields_are_schema_optional() -> None:
|
||||
required = set(reporting_tool.create_vulnerability_report.params_json_schema["required"])
|
||||
# every always-required essential is still schema-required
|
||||
assert required >= _SCHEMA_REQUIRED
|
||||
# no flag-gated field is schema-required (the model may omit it; the flag-aware
|
||||
# runtime validator decides whether the omission is allowed)
|
||||
assert not (_SCHEMA_OPTIONAL & required)
|
||||
Loading…
Add table
Reference in a new issue