mirror of
https://github.com/usestrix/strix.git
synced 2026-08-28 05:25:00 +00:00
feat(reporting): require repo-relative manifest_path on dependency CVE findings
This commit is contained in:
parent
97336d53e4
commit
72cb15a20a
3 changed files with 114 additions and 1 deletions
|
|
@ -200,6 +200,12 @@ findings and rejects empty PoC fields):
|
||||||
- `package_ecosystem` — normalized ecosystem from `.Results[].Type` (lowercased,
|
- `package_ecosystem` — normalized ecosystem from `.Results[].Type` (lowercased,
|
||||||
e.g. `npm`, `pypi`, `go`, `maven`, `rubygems`, `cargo`) (required).
|
e.g. `npm`, `pypi`, `go`, `maven`, `rubygems`, `cargo`) (required).
|
||||||
- `fixed_version` — `FixedVersion` (leave empty only if no fix is published).
|
- `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` /
|
- Reference the repo-relative `Target` lockfile path in `description` /
|
||||||
`technical_analysis` (no leading slash) so the finding is traceable.
|
`technical_analysis` (no leading slash) so the finding is traceable.
|
||||||
- Put the concrete proof in `description` / `technical_analysis`: package name,
|
- Put the concrete proof in `description` / `technical_analysis`: package name,
|
||||||
|
|
|
||||||
|
|
@ -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(
|
def _build_dependency_metadata(
|
||||||
*,
|
*,
|
||||||
package_name: str,
|
package_name: str,
|
||||||
|
|
@ -738,6 +757,7 @@ def _build_dependency_metadata(
|
||||||
fixed_version: str | None,
|
fixed_version: str | None,
|
||||||
introduced_by: str | None,
|
introduced_by: str | None,
|
||||||
dependency_path: str | None,
|
dependency_path: str | None,
|
||||||
|
manifest_path: str | None = None,
|
||||||
reachability: str | None = None,
|
reachability: str | None = None,
|
||||||
reachability_evidence: str | None = None,
|
reachability_evidence: str | None = None,
|
||||||
) -> dict[str, str]:
|
) -> dict[str, str]:
|
||||||
|
|
@ -747,6 +767,8 @@ def _build_dependency_metadata(
|
||||||
}
|
}
|
||||||
if package_ecosystem and package_ecosystem.strip():
|
if package_ecosystem and package_ecosystem.strip():
|
||||||
metadata["package_ecosystem"] = 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():
|
if fixed_version and fixed_version.strip():
|
||||||
metadata["fixed_version"] = fixed_version.strip()
|
metadata["fixed_version"] = fixed_version.strip()
|
||||||
if introduced_by and introduced_by.strip():
|
if introduced_by and introduced_by.strip():
|
||||||
|
|
@ -827,6 +849,7 @@ async def _do_create_dependency( # noqa: PLR0912
|
||||||
fix_effort: str,
|
fix_effort: str,
|
||||||
introduced_by: str | None = None,
|
introduced_by: str | None = None,
|
||||||
dependency_path: str | None = None,
|
dependency_path: str | None = None,
|
||||||
|
manifest_path: str | None = None,
|
||||||
reachability: str = "unknown",
|
reachability: str = "unknown",
|
||||||
reachability_evidence: str | None = None,
|
reachability_evidence: str | None = None,
|
||||||
agent_id: 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)}"
|
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()
|
reachability = (reachability or "unknown").strip().lower()
|
||||||
if reachability not in _VALID_REACHABILITY:
|
if reachability not in _VALID_REACHABILITY:
|
||||||
errors.append(
|
errors.append(
|
||||||
|
|
@ -897,6 +924,7 @@ async def _do_create_dependency( # noqa: PLR0912
|
||||||
fixed_version=fixed_version,
|
fixed_version=fixed_version,
|
||||||
introduced_by=introduced_by,
|
introduced_by=introduced_by,
|
||||||
dependency_path=dependency_path,
|
dependency_path=dependency_path,
|
||||||
|
manifest_path=manifest_path,
|
||||||
reachability=reachability,
|
reachability=reachability,
|
||||||
reachability_evidence=reachability_evidence,
|
reachability_evidence=reachability_evidence,
|
||||||
)
|
)
|
||||||
|
|
@ -1001,6 +1029,7 @@ async def create_dependency_report(
|
||||||
remediation_steps: str,
|
remediation_steps: str,
|
||||||
assumptions: str,
|
assumptions: str,
|
||||||
package_ecosystem: str,
|
package_ecosystem: str,
|
||||||
|
manifest_path: str | None = None,
|
||||||
fixed_version: str | None = None,
|
fixed_version: str | None = None,
|
||||||
cwe: str | None = None,
|
cwe: str | None = None,
|
||||||
technical_analysis: 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.
|
to the vulnerable package, joined with `` > `` (e.g.
|
||||||
``express@4.18.1 > body-parser@1.20.0 > qs@6.10.2``). Omit
|
``express@4.18.1 > body-parser@1.20.0 > qs@6.10.2``). Omit
|
||||||
for direct dependencies.
|
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
|
reachability: Usage-evidence level from static analysis — one of
|
||||||
``not_imported`` / ``imported`` / ``vulnerable_symbol_used`` /
|
``not_imported`` / ``imported`` / ``vulnerable_symbol_used`` /
|
||||||
``reachable_call_path`` / ``unknown``. Claim only what the
|
``reachable_call_path`` / ``unknown``. Claim only what the
|
||||||
|
|
@ -1116,6 +1152,7 @@ async def create_dependency_report(
|
||||||
fix_effort=fix_effort,
|
fix_effort=fix_effort,
|
||||||
introduced_by=introduced_by,
|
introduced_by=introduced_by,
|
||||||
dependency_path=dependency_path,
|
dependency_path=dependency_path,
|
||||||
|
manifest_path=manifest_path,
|
||||||
reachability=reachability,
|
reachability=reachability,
|
||||||
reachability_evidence=reachability_evidence,
|
reachability_evidence=reachability_evidence,
|
||||||
agent_id=agent_id,
|
agent_id=agent_id,
|
||||||
|
|
|
||||||
|
|
@ -141,6 +141,7 @@ async def test_dependency_report_sets_class_and_metadata(report_state: ReportSta
|
||||||
remediation_steps="Upgrade to 4.17.21.",
|
remediation_steps="Upgrade to 4.17.21.",
|
||||||
assumptions="Assumes the template sink is reachable.",
|
assumptions="Assumes the template sink is reachable.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="4.17.21",
|
fixed_version="4.17.21",
|
||||||
cwe="CWE-94",
|
cwe="CWE-94",
|
||||||
advisory_cvss=7.2,
|
advisory_cvss=7.2,
|
||||||
|
|
@ -160,6 +161,7 @@ async def test_dependency_report_sets_class_and_metadata(report_state: ReportSta
|
||||||
"package_name": "lodash",
|
"package_name": "lodash",
|
||||||
"installed_version": "4.17.20",
|
"installed_version": "4.17.20",
|
||||||
"package_ecosystem": "npm",
|
"package_ecosystem": "npm",
|
||||||
|
"manifest_path": "package-lock.json",
|
||||||
"fixed_version": "4.17.21",
|
"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.",
|
remediation_steps="Upgrade express to 4.18.2, which resolves qs 6.11.0.",
|
||||||
assumptions="qs parses all incoming query strings by default.",
|
assumptions="qs parses all incoming query strings by default.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="6.10.3",
|
fixed_version="6.10.3",
|
||||||
cwe="CWE-1321",
|
cwe="CWE-1321",
|
||||||
advisory_cvss=7.5,
|
advisory_cvss=7.5,
|
||||||
|
|
@ -213,6 +216,7 @@ async def test_dependency_report_omits_blank_chain_fields(report_state: ReportSt
|
||||||
remediation_steps="Upgrade.",
|
remediation_steps="Upgrade.",
|
||||||
assumptions="Assumptions.",
|
assumptions="Assumptions.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version=None,
|
fixed_version=None,
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=5.0,
|
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.",
|
remediation_steps="Upgrade to 1.0.1.",
|
||||||
assumptions="Assumes the package is included in deployed builds.",
|
assumptions="Assumes the package is included in deployed builds.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=0.0,
|
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.",
|
remediation_steps="Upgrade to 4.17.21.",
|
||||||
assumptions="Assumes the template sink is reachable.",
|
assumptions="Assumes the template sink is reachable.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="4.17.21",
|
fixed_version="4.17.21",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=7.2,
|
advisory_cvss=7.2,
|
||||||
|
|
@ -303,6 +309,7 @@ async def test_dependency_report_rejects_reachability_without_evidence(
|
||||||
remediation_steps="Upgrade.",
|
remediation_steps="Upgrade.",
|
||||||
assumptions="Assumptions.",
|
assumptions="Assumptions.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=5.0,
|
advisory_cvss=5.0,
|
||||||
|
|
@ -330,6 +337,7 @@ async def test_dependency_report_rejects_unknown_reachability_level(
|
||||||
remediation_steps="Upgrade.",
|
remediation_steps="Upgrade.",
|
||||||
assumptions="Assumptions.",
|
assumptions="Assumptions.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=5.0,
|
advisory_cvss=5.0,
|
||||||
|
|
@ -356,6 +364,7 @@ async def test_dependency_report_omits_unknown_reachability(report_state: Report
|
||||||
remediation_steps="Upgrade.",
|
remediation_steps="Upgrade.",
|
||||||
assumptions="Analysis was inconclusive.",
|
assumptions="Analysis was inconclusive.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=5.0,
|
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.",
|
remediation_steps="Upgrade to 1.0.1.",
|
||||||
assumptions="Assumes the package ships in deployed builds.",
|
assumptions="Assumes the package ships in deployed builds.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=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.",
|
remediation_steps="Upgrade to 1.0.1.",
|
||||||
assumptions="Assumes the package is included in deployed builds.",
|
assumptions="Assumes the package is included in deployed builds.",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=0.0,
|
advisory_cvss=0.0,
|
||||||
|
|
@ -453,6 +464,7 @@ async def test_dependency_report_dedupe_candidate_includes_dependency_metadata(
|
||||||
"package_name": "sample",
|
"package_name": "sample",
|
||||||
"installed_version": "1.0.0",
|
"installed_version": "1.0.0",
|
||||||
"package_ecosystem": "npm",
|
"package_ecosystem": "npm",
|
||||||
|
"manifest_path": "package-lock.json",
|
||||||
"fixed_version": "1.0.1",
|
"fixed_version": "1.0.1",
|
||||||
},
|
},
|
||||||
"technical_analysis": None,
|
"technical_analysis": None,
|
||||||
|
|
@ -471,6 +483,7 @@ async def test_dependency_report_rejects_bad_cve(report_state: ReportState) -> N
|
||||||
remediation_steps="r",
|
remediation_steps="r",
|
||||||
assumptions="a",
|
assumptions="a",
|
||||||
package_ecosystem="npm",
|
package_ecosystem="npm",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version=None,
|
fixed_version=None,
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=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.",
|
remediation_steps="Upgrade to 1.0.1.",
|
||||||
assumptions="Assumes the package is included in deployed builds.",
|
assumptions="Assumes the package is included in deployed builds.",
|
||||||
package_ecosystem="",
|
package_ecosystem="",
|
||||||
|
manifest_path="package-lock.json",
|
||||||
fixed_version="1.0.1",
|
fixed_version="1.0.1",
|
||||||
cwe=None,
|
cwe=None,
|
||||||
advisory_cvss=0.0,
|
advisory_cvss=0.0,
|
||||||
|
|
@ -505,6 +519,62 @@ async def test_dependency_report_requires_ecosystem(report_state: ReportState) -
|
||||||
assert not report_state.vulnerability_reports
|
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:
|
def test_dedupe_comparison_preserves_cve_identity() -> None:
|
||||||
cleaned = _prepare_report_for_comparison(
|
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"]
|
dep_props = create_dependency_report.params_json_schema["properties"]
|
||||||
for field in ("package_name", "installed_version", "cve", "advisory_cvss"):
|
for field in ("package_name", "installed_version", "cve", "advisory_cvss"):
|
||||||
assert field in dep_props
|
assert field in dep_props
|
||||||
for field in ("reachability", "reachability_evidence"):
|
for field in ("reachability", "reachability_evidence", "manifest_path"):
|
||||||
assert field in dep_props
|
assert field in dep_props
|
||||||
dep_required = create_dependency_report.params_json_schema["required"]
|
dep_required = create_dependency_report.params_json_schema["required"]
|
||||||
assert "package_ecosystem" in dep_required
|
assert "package_ecosystem" in dep_required
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue