From 6727d0ba15aa917ed3a67dc954a3c7a8bcbdbda2 Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Sat, 3 Oct 2026 19:06:37 +0100 Subject: [PATCH 1/8] feat(reporting): STRIX_REQUIRE_POC flag to file findings without a runnable PoC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_vulnerability_report requires a runnable PoC/exploit by default (upstream's false-positive-reduction intent, unchanged). But many real, gradable weaknesses have no practical exploit — hardcoded secrets, missing hardening, defence-in-depth gaps, contrived-only issues — and in SAST/SCA or downstream-triage workflows they should be recorded and graded (down to informational via the existing CVSS "none" -> info path), not silently dropped because no exploit exists. STRIX_REQUIRE_POC (default on) gates the mandatory poc_script_code field. Set it to a false-y value (0/false/no/off) for "raw findings" use: poc_script_code may then be blank while every other required field is still enforced. The default rejection message now points at the flag so the escape hatch is discoverable at the point of friction (and notes it's apt when a downstream tool triages/validates the findings). Co-Authored-By: Claude Opus 4.8 --- strix/tools/reporting/tool.py | 38 ++++++++++++++++++--- tests/test_reporting_poc_optional.py | 49 ++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 tests/test_reporting_poc_optional.py diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 3faf7fbd6..f9353051b 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -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 @@ -161,7 +162,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 +179,33 @@ _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", + ) + + 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. + + When ``STRIX_REQUIRE_POC`` is disabled, a blank ``poc_script_code`` is allowed so + exploit-less findings can still be filed (and graded on their merits). + """ + required = _REQUIRED_FIELDS + if not _poc_required(): + required = {k: v for k, v in _REQUIRED_FIELDS.items() if k != _POC_OPTIONAL_FIELD} + return [msg for name, msg in required.items() if not str(fields.get(name) or "").strip()] def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]]: diff --git a/tests/test_reporting_poc_optional.py b/tests/test_reporting_poc_optional.py new file mode 100644 index 000000000..0f41d76d8 --- /dev/null +++ b/tests/test_reporting_poc_optional.py @@ -0,0 +1,49 @@ +"""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 From 7c4b0a1284ed283e6277c3e8831def04ac652d4b Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Sun, 4 Oct 2026 23:12:06 +0100 Subject: [PATCH 2/8] feat(runtime): STRIX_MOUNT_READONLY to bind local targets read-only Review/localization scans should inspect, not modify, the code under review. Without this, a local target mounts writable and an agent can wipe or "fix" the target instead of reporting on it (observed: a weak orchestrator wiped the target file via a botched patch, then looped reconstructing it instead of reviewing). STRIX_MOUNT_READONLY=1 sets read_only on local-source bind mounts (the docker layer already supports per-mount read_only; this just wires it). Per-target read_only is still honored. Default off = unchanged behavior. Co-Authored-By: Claude Opus 4.8 --- strix/interface/utils.py | 14 +++++++++- tests/test_mount_readonly.py | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/test_mount_readonly.py diff --git a/strix/interface/utils.py b/strix/interface/utils.py index 7ab55d8ee..64e4a86ed 100644 --- a/strix/interface/utils.py +++ b/strix/interface/utils.py @@ -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, } ) diff --git a/tests/test_mount_readonly.py b/tests/test_mount_readonly.py new file mode 100644 index 000000000..108835211 --- /dev/null +++ b/tests/test_mount_readonly.py @@ -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 From 1dcd022734e482c333432a708ea33d30dbd34108 Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Mon, 5 Oct 2026 00:36:09 +0100 Subject: [PATCH 3/8] fix(reporting): normalize sandbox-absolute code_location paths instead of rejecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_vulnerability_report rejected absolute code_locations ("file path must be relative, got absolute"). But agents see the target at /workspace//... and naturally report that absolute path, so every file attempt was silently dropped: findings detected, nothing filed (findings=N filed_reports=0) — observed as a weak model "describing but not filing" and an endless re-file churn (42 validation failures in one local run). Normalize absolute paths under the workspace root to relative in _normalize_code_locations (strip /workspace prefix) so they validate and file; paths genuinely outside the workspace are still rejected. Likely the root cause of usestrix/strix#1439 (agents detect findings but filed_reports=0 on local backends). The finish/agent_finish filing guards address the symptom; this fixes the source. Refs usestrix/strix#1439 Co-Authored-By: Claude Opus 4.8 --- strix/tools/reporting/tool.py | 29 +++++++++++++++++ tests/test_code_location_paths.py | 54 +++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 tests/test_code_location_paths.py diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index f9353051b..de60ef7f6 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -51,6 +51,33 @@ _CODE_LOCATION_FIELDS = ( ) +# Sandbox mount root (mirrors strix.runtime.session_manager._WORKSPACE_ROOT). Agents see +# the target at /workspace//... 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 relative to the workspace root. + + Agents report the absolute path they see in the sandbox (e.g. + ``/workspace//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 prefix so the path validates 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: + return str(p.relative_to(_WORKSPACE_ROOT)) + except ValueError: + return path + + def _validate_file_path(path: str) -> str | None: if not path or not path.strip(): return "file path cannot be empty" @@ -85,6 +112,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: diff --git a/tests/test_code_location_paths.py b/tests/test_code_location_paths.py new file mode 100644 index 000000000..753efbf5a --- /dev/null +++ b/tests/test_code_location_paths.py @@ -0,0 +1,54 @@ +"""code_locations accept the sandbox-absolute path agents naturally report. + +Agents see the target at /workspace//... 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 so filing +succeeds, 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_relative() -> None: + assert ( + _to_workspace_relative("/workspace/b8bLSpkh/src/picklescan/scanner.py") + == "b8bLSpkh/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_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"] == "b8bLSpkh/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] From 0d6fdeb18d7583bb5bd34d94110e1573a3ba035f Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Mon, 5 Oct 2026 04:09:30 +0100 Subject: [PATCH 4/8] feat(reporting): log create_vulnerability_report validation failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rejected filing was returned to the agent but never logged, so a finding that was detected but could not be filed (validation failed on a required field) looked like silence — invisible in strix.log, which made findings=N filed_reports=0 very hard to diagnose. Log the rejected title + the specific validation errors at WARNING so filing failures are observable. Refs usestrix/strix#1439 Co-Authored-By: Claude Opus 4.8 --- strix/tools/reporting/tool.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index de60ef7f6..b6f60cd65 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -908,11 +908,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: From 71e88c337455d8bf61d73716c7d7a9aa302cb8b0 Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Mon, 5 Oct 2026 07:22:23 +0100 Subject: [PATCH 5/8] feat(reporting): STRIX_RELAX_FINDING_FIELDS to relax non-essential finding fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_vulnerability_report hard-requires ~11 fields (title, description, impact, target, technical_analysis, poc_description, poc_script_code, remediation_steps, evidence, assumptions, counterevidence, severity_change_conditions). A weak local model that detects a real finding but omits one analysis field has the whole filing rejected -> the finding is never recorded (findings=N filed_reports=0), the create-side driver of usestrix/strix#1439. STRIX_RELAX_FINDING_FIELDS relaxes a subset of the ANALYSIS fields from required to optional (empty-but-relaxed is logged, not blocked), keeping the essentials — title, description, impact, target, code_locations, cvss, confidence — required, so a detected finding is RECORDED rather than lost. "1"/"all" relaxes the whole set; a comma-list names a subset; essentials can never be relaxed. Default off = unchanged strict behaviour. Mirrors the existing STRIX_REQUIRE_POC relaxation. Refs usestrix/strix#1439 Co-Authored-By: Claude Opus 4.8 --- strix/tools/reporting/tool.py | 56 +++++++++++++++++--- tests/test_relax_finding_fields.py | 85 ++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 tests/test_relax_finding_fields.py diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index b6f60cd65..213907927 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -225,16 +225,55 @@ def _poc_required() -> bool: ) +# 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()} + return frozenset(named & _RELAXABLE_FIELDS) + + def _validate_required_text(fields: dict[str, str]) -> list[str]: """Report every ``_REQUIRED_FIELDS`` entry that arrived blank. - When ``STRIX_REQUIRE_POC`` is disabled, a blank ``poc_script_code`` is allowed so - exploit-less findings can still be filed (and graded on their merits). + ``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. """ - required = _REQUIRED_FIELDS + 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_FIELDS.items() if k != _POC_OPTIONAL_FIELD} - return [msg for name, msg in required.items() if not str(fields.get(name) or "").strip()] + 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]]: @@ -366,13 +405,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" diff --git a/tests/test_relax_finding_fields.py b/tests/test_relax_finding_fields.py new file mode 100644 index 000000000..f9e7241d8 --- /dev/null +++ b/tests/test_relax_finding_fields.py @@ -0,0 +1,85 @@ +"""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 + +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_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 From fbca4ba69bd94c49b73328a25faa720701ad1bdb Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Thu, 15 Oct 2026 13:57:24 +0100 Subject: [PATCH 6/8] fix(reporting): warn on unknown STRIX_RELAX_FINDING_FIELDS names; clarify _WORKSPACE_ROOT mirror - _relaxed_fields now logs a warning for unrecognized field names instead of silently dropping them (a typo looked like it worked). - Explain why _WORKSPACE_ROOT is mirrored from runtime.session_manager rather than imported: that module loads the sandbox/Docker machinery and this tool is otherwise runtime-free. Co-Authored-By: Claude Opus 4.8 --- strix/tools/reporting/tool.py | 16 +++++++++++++--- tests/test_relax_finding_fields.py | 12 ++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 213907927..4223f1f99 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -51,9 +51,12 @@ _CODE_LOCATION_FIELDS = ( ) -# Sandbox mount root (mirrors strix.runtime.session_manager._WORKSPACE_ROOT). Agents see -# the target at /workspace//... and naturally report code_locations with that -# absolute path; the report format wants it relative. +# 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//... and naturally report +# code_locations with that absolute path; the report format wants it relative. _WORKSPACE_ROOT = "/workspace" @@ -251,6 +254,13 @@ def _relaxed_fields() -> frozenset[str]: 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) diff --git a/tests/test_relax_finding_fields.py b/tests/test_relax_finding_fields.py index f9e7241d8..4b8a75376 100644 --- a/tests/test_relax_finding_fields.py +++ b/tests/test_relax_finding_fields.py @@ -7,6 +7,7 @@ analysis field — the create-side cause of findings-detected-but-never-filed from __future__ import annotations +import logging from typing import TYPE_CHECKING from strix.tools.reporting.tool import ( @@ -54,6 +55,17 @@ def test_relaxed_fields_parsing(monkeypatch: pytest.MonkeyPatch) -> None: 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") From 5379e243a74d12e8877f0045cc94ac1c17d15722 Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Thu, 22 Oct 2026 22:06:59 +0100 Subject: [PATCH 7/8] fix(core): tell the agent a read-only local target is read-only The root task described every local target as "mounted live and writable", even under STRIX_MOUNT_READONLY. Co-Authored-By: Claude Opus 5.5 --- strix/core/inputs.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/strix/core/inputs.py b/strix/core/inputs.py index 7bb2b72c7..db18d487a 100644 --- a/strix/core/inputs.py +++ b/strix/core/inputs.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import os from typing import TYPE_CHECKING, Any from agents.model_settings import ModelSettings @@ -27,6 +28,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/"): @@ -134,8 +147,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', '')}") From 7eccdf8b9b231a3c4b1903a577bf2b2d4758279f Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Fri, 23 Oct 2026 18:10:59 +0100 Subject: [PATCH 8/8] fix(reporting): repo-root-relative code paths + schema-optional gated fields Address the two Greptile findings on #1578: - _to_workspace_relative stripped only /workspace, leaving the per-target mount subdir (e.g. b8bLSpkh/src/x.py). SARIF/report consumers want a repo-root-relative path (strix/report/sarif.py: "File locations must be repo-relative"); the subdir prefix breaks GitHub code-scanning / IDE viewers rooted at the repo. Strip the mount subdir too -> src/x.py. - The flag-gated report fields (poc_script_code under STRIX_REQUIRE_POC, the analysis fields under STRIX_RELAX_FINDING_FIELDS) were declared as schema-required args, so the model was forced to emit (and fabricate) a PoC even when the operator disabled the requirement. Default them to "" (keyword-only) so they are schema-OPTIONAL; the existing flag-aware runtime validators remain the single source of required-ness. The seven always-required essentials stay schema-required. Tests: repo-relative + bare-subdir cases; schema-optional assertion over the generated tool schema. 399 reporting/registration tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 --- strix/tools/reporting/tool.py | 52 +++++++++++++++++++++------- tests/test_code_location_paths.py | 19 +++++++--- tests/test_reporting_poc_optional.py | 31 +++++++++++++++++ 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 4223f1f99..dd98b5091 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -61,14 +61,15 @@ _WORKSPACE_ROOT = "/workspace" def _to_workspace_relative(path: str) -> str: - """Make a sandbox-absolute code-location path relative to the workspace root. + """Make a sandbox-absolute code-location path repo-root-relative. Agents report the absolute path they see in the sandbox (e.g. ``/workspace//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 prefix so the path validates and files. A path genuinely outside - the workspace is left unchanged for the validator to reject. + 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 @@ -76,9 +77,20 @@ def _to_workspace_relative(path: str) -> str: if not p.is_absolute(): return path try: - return str(p.relative_to(_WORKSPACE_ROOT)) + rel = p.relative_to(_WORKSPACE_ROOT) except ValueError: return path + # The target is mounted at /workspace//..., where 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 / prefix + # breaks that mapping. Drop the mount component too. A bare /workspace/ + # (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: @@ -1102,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, @@ -1378,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. diff --git a/tests/test_code_location_paths.py b/tests/test_code_location_paths.py index 753efbf5a..1d5eda736 100644 --- a/tests/test_code_location_paths.py +++ b/tests/test_code_location_paths.py @@ -3,8 +3,9 @@ Agents see the target at /workspace//... 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 so filing -succeeds, while paths genuinely outside the workspace are still rejected. +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 @@ -16,10 +17,12 @@ from strix.tools.reporting.tool import ( ) -def test_workspace_absolute_becomes_relative() -> None: +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") - == "b8bLSpkh/src/picklescan/scanner.py" + == "src/picklescan/scanner.py" ) @@ -31,6 +34,12 @@ 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/ 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 = [ { @@ -42,7 +51,7 @@ def test_absolute_workspace_location_normalizes_and_validates() -> None: ] normalized = _normalize_code_locations(loc) assert normalized is not None - assert normalized[0]["file"] == "b8bLSpkh/src/picklescan/scanner.py" + assert normalized[0]["file"] == "src/picklescan/scanner.py" assert _validate_code_locations(normalized) == [] # files cleanly now diff --git a/tests/test_reporting_poc_optional.py b/tests/test_reporting_poc_optional.py index 0f41d76d8..745135da7 100644 --- a/tests/test_reporting_poc_optional.py +++ b/tests/test_reporting_poc_optional.py @@ -47,3 +47,34 @@ def test_flag_falsey_and_truthy_values(monkeypatch: pytest.MonkeyPatch) -> None: 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)