From 72cb15a20af962503ac02d0ca475bae0668a825f Mon Sep 17 00:00:00 2001 From: Ahmed Allam Date: Wed, 5 Aug 2026 22:50:04 +0000 Subject: [PATCH] feat(reporting): require repo-relative manifest_path on dependency CVE findings --- .../skills/custom/dependency_cve_scanning.md | 6 ++ strix/tools/reporting/tool.py | 37 ++++++++++ tests/test_reporting_fields.py | 72 ++++++++++++++++++- 3 files changed, 114 insertions(+), 1 deletion(-) diff --git a/strix/skills/custom/dependency_cve_scanning.md b/strix/skills/custom/dependency_cve_scanning.md index 59e37ecc..766c41e1 100644 --- a/strix/skills/custom/dependency_cve_scanning.md +++ b/strix/skills/custom/dependency_cve_scanning.md @@ -200,6 +200,12 @@ findings and rejects empty PoC fields): - `package_ecosystem` — normalized ecosystem from `.Results[].Type` (lowercased, e.g. `npm`, `pypi`, `go`, `maven`, `rubygems`, `cargo`) (required). - `fixed_version` — `FixedVersion` (leave empty only if no fix is published). + - `manifest_path` — the repo-relative `Target` lockfile/manifest path + (required). Strip any scan-workspace or repo checkout directory prefix so + the path is relative to the repository root (e.g. `package-lock.json`, + `services/api/pom.xml`); the tool rejects absolute paths and `..` segments. + This binds the finding to the exact file so remediation can target the + right repository. - Reference the repo-relative `Target` lockfile path in `description` / `technical_analysis` (no leading slash) so the finding is traceable. - Put the concrete proof in `description` / `technical_analysis`: package name, diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index e2c08c1e..12dd2046 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -730,6 +730,25 @@ _VALID_REACHABILITY = frozenset( ) +def _validate_manifest_path(manifest_path: str | None) -> str | None: + """Return an error message when manifest_path is missing or unsafe.""" + path = (manifest_path or "").strip() + if not path: + return ( + "manifest_path is required: pass the repo-relative path of the " + "lockfile/manifest where the vulnerable version was observed " + "(trivy's Target, e.g. 'package-lock.json' or " + "'services/api/pom.xml'). It binds the finding to its exact file " + "so remediation can target the right repository." + ) + if path.startswith("/") or "\\" in path or path.split("/")[0].endswith(":"): + return f"manifest_path must be a relative path within the repository, got {path!r}" + segments = path.split("/") + if any(segment in ("", ".", "..") for segment in segments): + return f"manifest_path must not contain empty, '.', or '..' segments, got {path!r}" + return None + + def _build_dependency_metadata( *, package_name: str, @@ -738,6 +757,7 @@ def _build_dependency_metadata( fixed_version: str | None, introduced_by: str | None, dependency_path: str | None, + manifest_path: str | None = None, reachability: str | None = None, reachability_evidence: str | None = None, ) -> dict[str, str]: @@ -747,6 +767,8 @@ def _build_dependency_metadata( } if package_ecosystem and package_ecosystem.strip(): metadata["package_ecosystem"] = package_ecosystem.strip() + if manifest_path and manifest_path.strip(): + metadata["manifest_path"] = manifest_path.strip() if fixed_version and fixed_version.strip(): metadata["fixed_version"] = fixed_version.strip() if introduced_by and introduced_by.strip(): @@ -827,6 +849,7 @@ async def _do_create_dependency( # noqa: PLR0912 fix_effort: str, introduced_by: str | None = None, dependency_path: str | None = None, + manifest_path: str | None = None, reachability: str = "unknown", reachability_evidence: str | None = None, agent_id: str | None = None, @@ -865,6 +888,10 @@ async def _do_create_dependency( # noqa: PLR0912 f"Invalid fix_effort: {fix_effort!r}. Must be one of: {sorted(_VALID_FIX_EFFORT)}" ) + manifest_err = _validate_manifest_path(manifest_path) + if manifest_err: + errors.append(manifest_err) + reachability = (reachability or "unknown").strip().lower() if reachability not in _VALID_REACHABILITY: errors.append( @@ -897,6 +924,7 @@ async def _do_create_dependency( # noqa: PLR0912 fixed_version=fixed_version, introduced_by=introduced_by, dependency_path=dependency_path, + manifest_path=manifest_path, reachability=reachability, reachability_evidence=reachability_evidence, ) @@ -1001,6 +1029,7 @@ async def create_dependency_report( remediation_steps: str, assumptions: str, package_ecosystem: str, + manifest_path: str | None = None, fixed_version: str | None = None, cwe: str | None = None, technical_analysis: str | None = None, @@ -1087,6 +1116,13 @@ async def create_dependency_report( to the vulnerable package, joined with `` > `` (e.g. ``express@4.18.1 > body-parser@1.20.0 > qs@6.10.2``). Omit for direct dependencies. + manifest_path: **Required.** The repo-relative path of the + lockfile/manifest where the vulnerable version was observed — + trivy's ``Target`` (e.g. ``package-lock.json``, + ``services/api/pom.xml``). Strip any scan-workspace or repo + checkout directory prefix so the path is relative to the + repository root. This binds the finding to its exact file so + remediation can target the right repository. reachability: Usage-evidence level from static analysis — one of ``not_imported`` / ``imported`` / ``vulnerable_symbol_used`` / ``reachable_call_path`` / ``unknown``. Claim only what the @@ -1116,6 +1152,7 @@ async def create_dependency_report( fix_effort=fix_effort, introduced_by=introduced_by, dependency_path=dependency_path, + manifest_path=manifest_path, reachability=reachability, reachability_evidence=reachability_evidence, agent_id=agent_id, diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index 027c0a9f..6eda5667 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -141,6 +141,7 @@ async def test_dependency_report_sets_class_and_metadata(report_state: ReportSta remediation_steps="Upgrade to 4.17.21.", assumptions="Assumes the template sink is reachable.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="4.17.21", cwe="CWE-94", advisory_cvss=7.2, @@ -160,6 +161,7 @@ async def test_dependency_report_sets_class_and_metadata(report_state: ReportSta "package_name": "lodash", "installed_version": "4.17.20", "package_ecosystem": "npm", + "manifest_path": "package-lock.json", "fixed_version": "4.17.21", } @@ -176,6 +178,7 @@ async def test_dependency_report_records_transitive_chain(report_state: ReportSt remediation_steps="Upgrade express to 4.18.2, which resolves qs 6.11.0.", assumptions="qs parses all incoming query strings by default.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="6.10.3", cwe="CWE-1321", advisory_cvss=7.5, @@ -213,6 +216,7 @@ async def test_dependency_report_omits_blank_chain_fields(report_state: ReportSt remediation_steps="Upgrade.", assumptions="Assumptions.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version=None, cwe=None, advisory_cvss=5.0, @@ -241,6 +245,7 @@ async def test_dependency_report_with_zero_cvss_remains_low_severity( remediation_steps="Upgrade to 1.0.1.", assumptions="Assumes the package is included in deployed builds.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=0.0, @@ -267,6 +272,7 @@ async def test_dependency_report_records_reachability(report_state: ReportState) remediation_steps="Upgrade to 4.17.21.", assumptions="Assumes the template sink is reachable.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="4.17.21", cwe=None, advisory_cvss=7.2, @@ -303,6 +309,7 @@ async def test_dependency_report_rejects_reachability_without_evidence( remediation_steps="Upgrade.", assumptions="Assumptions.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=5.0, @@ -330,6 +337,7 @@ async def test_dependency_report_rejects_unknown_reachability_level( remediation_steps="Upgrade.", assumptions="Assumptions.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=5.0, @@ -356,6 +364,7 @@ async def test_dependency_report_omits_unknown_reachability(report_state: Report remediation_steps="Upgrade.", assumptions="Analysis was inconclusive.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=5.0, @@ -381,6 +390,7 @@ async def test_dependency_report_requires_advisory_cvss(report_state: ReportStat remediation_steps="Upgrade to 1.0.1.", assumptions="Assumes the package ships in deployed builds.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=None, @@ -436,6 +446,7 @@ async def test_dependency_report_dedupe_candidate_includes_dependency_metadata( remediation_steps="Upgrade to 1.0.1.", assumptions="Assumes the package is included in deployed builds.", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=0.0, @@ -453,6 +464,7 @@ async def test_dependency_report_dedupe_candidate_includes_dependency_metadata( "package_name": "sample", "installed_version": "1.0.0", "package_ecosystem": "npm", + "manifest_path": "package-lock.json", "fixed_version": "1.0.1", }, "technical_analysis": None, @@ -471,6 +483,7 @@ async def test_dependency_report_rejects_bad_cve(report_state: ReportState) -> N remediation_steps="r", assumptions="a", package_ecosystem="npm", + manifest_path="package-lock.json", fixed_version=None, cwe=None, advisory_cvss=None, @@ -493,6 +506,7 @@ async def test_dependency_report_requires_ecosystem(report_state: ReportState) - remediation_steps="Upgrade to 1.0.1.", assumptions="Assumes the package is included in deployed builds.", package_ecosystem="", + manifest_path="package-lock.json", fixed_version="1.0.1", cwe=None, advisory_cvss=0.0, @@ -505,6 +519,62 @@ async def test_dependency_report_requires_ecosystem(report_state: ReportState) - assert not report_state.vulnerability_reports +async def test_dependency_report_requires_manifest_path(report_state: ReportState) -> None: + result = await _do_create_dependency( + title="CVE-2024-0001 in sample 1.0.0", + description="Published advisory affects the pinned version.", + target="repo/package.json", + cve="CVE-2024-0001", + package_name="sample", + installed_version="1.0.0", + impact="Low-impact dependency advisory.", + remediation_steps="Upgrade to 1.0.1.", + assumptions="Assumes the package is included in deployed builds.", + package_ecosystem="npm", + manifest_path=None, + fixed_version="1.0.1", + cwe=None, + advisory_cvss=5.0, + technical_analysis=None, + fix_effort="low", + ) + + assert result["success"] is False + assert any("manifest_path is required" in error for error in result["errors"]) + assert not report_state.vulnerability_reports + + +@pytest.mark.parametrize( + "bad_path", + ["/etc/passwd", "..\\pom.xml", "services/../pom.xml", "./package.json", "C:/repo/pom.xml"], +) +async def test_dependency_report_rejects_unsafe_manifest_path( + report_state: ReportState, bad_path: str +) -> None: + result = await _do_create_dependency( + title="CVE-2024-0001 in sample 1.0.0", + description="Published advisory affects the pinned version.", + target="repo/package.json", + cve="CVE-2024-0001", + package_name="sample", + installed_version="1.0.0", + impact="Low-impact dependency advisory.", + remediation_steps="Upgrade to 1.0.1.", + assumptions="Assumes the package is included in deployed builds.", + package_ecosystem="npm", + manifest_path=bad_path, + fixed_version="1.0.1", + cwe=None, + advisory_cvss=5.0, + technical_analysis=None, + fix_effort="low", + ) + + assert result["success"] is False + assert any("manifest_path" in error for error in result["errors"]) + assert not report_state.vulnerability_reports + + def test_dedupe_comparison_preserves_cve_identity() -> None: cleaned = _prepare_report_for_comparison( { @@ -736,7 +806,7 @@ def test_vuln_tool_exposes_new_params() -> None: dep_props = create_dependency_report.params_json_schema["properties"] for field in ("package_name", "installed_version", "cve", "advisory_cvss"): assert field in dep_props - for field in ("reachability", "reachability_evidence"): + for field in ("reachability", "reachability_evidence", "manifest_path"): assert field in dep_props dep_required = create_dependency_report.params_json_schema["required"] assert "package_ecosystem" in dep_required