diff --git a/strix/core/inputs.py b/strix/core/inputs.py index b5dad68bc..c7151ff99 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 @@ -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', '')}") 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/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 3faf7fbd6..dd98b5091 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 @@ -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//... 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//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//..., 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: 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. diff --git a/tests/test_code_location_paths.py b/tests/test_code_location_paths.py new file mode 100644 index 000000000..1d5eda736 --- /dev/null +++ b/tests/test_code_location_paths.py @@ -0,0 +1,63 @@ +"""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 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/ 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] 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 diff --git a/tests/test_relax_finding_fields.py b/tests/test_relax_finding_fields.py new file mode 100644 index 000000000..4b8a75376 --- /dev/null +++ b/tests/test_relax_finding_fields.py @@ -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 diff --git a/tests/test_reporting_poc_optional.py b/tests/test_reporting_poc_optional.py new file mode 100644 index 000000000..745135da7 --- /dev/null +++ b/tests/test_reporting_poc_optional.py @@ -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)