Skip to content

fix(sarif): preserve source locations for endpoint findings #1111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aias00 wants to merge 2 commits into
base: main
Choose a base branch
from
+89 −41
Open

fix(sarif): preserve source locations for endpoint findings #1111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fca4dc
fix(sarif): preserve source locations for endpoint findings
Aias00 Aug 18, 2026
79c9173
fix(sarif): preserve synthetic alert identity
Aias00 Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 41 additions & 35 deletions strix/report/sarif.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
the run carries ``versionControlProvenance`` + ``automationDetails``
so code-scanning can bind alerts to the scanned commit and branch.
* Findings without safe locations still appear in the SARIF output,
anchored to SECURITY.md and flagged via
anchored to a stable, tracked repository file and flagged via
``properties.synthetic_location`` rather than being dropped silently.
"""

Expand All @@ -63,13 +63,15 @@

# Synthetic anchor for findings that have no safe code location. SARIF
# requires every result to carry at least one location, and GitHub
# code-scanning's UI handles locationless results unreliably. Anchoring
# to SECURITY.md keeps the result valid + visible while a
# ``properties.synthetic_location: true`` flag lets downstream tooling
# distinguish synthetic anchors from real source locations. Anchoring
# also lets the partialFingerprints + class-hash code path cover these
# findings instead of re-orphaning them on every run.
_SYNTHETIC_LOCATION_URI = "SECURITY.md"
# code-scanning's UI handles locationless results unreliably. Anchor those
# findings to README.md, which is tracked in every scanned checkout. The
# location is synthetic, so ``properties.synthetic_location: true`` lets
# tooling distinguish it from real source locations while retaining a
# code-scanning-compatible result location and stable fingerprint.
_SYNTHETIC_LOCATION_URI = "README.md"

@greptile-apps greptile-apps Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Synthetic URI breaks alert identity

When a locationless finding is re-uploaded after this change, _primary_fingerprint hashes README.md instead of the previous SECURITY.md anchor, causing code scanning to replace the existing alert and lose its prior dismissal or triage state.

Knowledge Base Used: Reporting and Output

Prompt To Fix With AI 
This is a comment left during a code review.
Path: strix/report/sarif.py
Line: 71

Comment:
**Synthetic URI breaks alert identity**

When a locationless finding is re-uploaded after this change, `_primary_fingerprint` hashes `README.md` instead of the previous `SECURITY.md` anchor, causing code scanning to replace the existing alert and lose its prior dismissal or triage state.

**Knowledge Base Used:** [Reporting and Output](https://app.greptile.com/strix-org-3/-/custom-context/knowledge-base/usestrix/strix/-/docs/reporting-and-output.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@Aias00 Aias00 Aug 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 79c9173. Synthetic results still emit README.md as the tracked display anchor, but their primary fingerprint now uses the legacy SECURITY.md anchor. Added a regression test confirming the emitted fingerprint preserves the historical identity.

# Preserve the pre-README synthetic identity so code-scanning retains prior
# alert dismissal and triage state across this display-location migration.
_LEGACY_SYNTHETIC_FINGERPRINT_URI = "SECURITY.md"


# SARIF only has three result levels; Strix's five severities collapse here.
Expand Down Expand Up @@ -209,8 +211,8 @@ def build_sarif_report(
can bind alerts to the scanned commit; it is omitted for URL / IP
(DAST) targets that have no repository.

Findings without safe source locations are anchored synthetically
to SECURITY.md and flagged via ``properties.synthetic_location``.
Findings without safe source locations are anchored synthetically to
a stable repository file and flagged via ``properties.synthetic_location``.
They're still emitted as proper SARIF results so they (a) flow
through code-scanning normally rather than being shunted into a
run-properties summary the UI can't render, and (b) carry the
Expand Down Expand Up @@ -446,8 +448,8 @@ def _build_result(
) -> dict[str, Any]:
"""Build one SARIF result using validated locations.

``is_synthetic`` flags results whose location is the SECURITY.md
anchor rather than a real code location — surfaces as
``is_synthetic`` flags results whose location is the synthetic
repository anchor rather than a real code location — surfaces as
``properties.synthetic_location: true`` so reviewers and downstream
tooling can distinguish anchored-locationless findings from
source-linked ones.
Expand Down Expand Up @@ -621,11 +623,9 @@ def _build_fixes(report: dict[str, Any]) -> list[dict[str, Any]] | None:
def _synthetic_location() -> dict[str, Any]:
"""Synthetic anchor for findings with no safe code location.

SARIF requires every result to carry at least one location, and
code-scanning's UI handles locationless results unreliably.
Anchoring to SECURITY.md gives the result a valid + visible
location; the result's ``properties.synthetic_location: true``
flag lets reviewers + tooling distinguish synthetic from real.
A SARIF artifact URI is resolved against the scanned checkout, not
Strix's run directory. ``README.md`` is a stable tracked file that
keeps locationless findings visible to code-scanning consumers.
"""
return {
"physicalLocation": {
Expand All @@ -638,15 +638,19 @@ def _build_locations(report: dict[str, Any]) -> tuple[list[dict[str, Any]], bool
"""Return ``(locations, is_synthetic, dropped_count)`` for a finding.

Physical locations come from validated ``code_locations``. When none
are safe, the result is anchored to SECURITY.md (``is_synthetic``).
An ``endpoint`` (typical of DAST findings) is added as a
``logicalLocations`` entry; a locationless, endpoint-less finding
gets a ``resource`` logical location carrying the target so the
finding keeps a human-meaningful anchor.
are safe, the result is anchored to a stable repository file
(``is_synthetic``). An ``endpoint`` (typical of DAST findings) is
added as a ``logicalLocations`` entry. A locationless, endpoint-less
finding gets a ``resource`` logical location carrying the target so
the finding keeps a human-meaningful anchor.
"""
physical, dropped_location_count = _build_physical_locations(report.get("code_locations"))
is_synthetic = not physical
locations: list[dict[str, Any]] = list(physical) if physical else [_synthetic_location()]

if physical:
locations: list[dict[str, Any]] = list(physical)
else:
locations = [_synthetic_location()]

endpoint = _string_value(report.get("endpoint"))
if endpoint:
Expand Down Expand Up @@ -838,22 +842,24 @@ def _primary_fingerprint(
- HTTP method + endpoint when present (BOLA/IDOR/missing-authz
findings carry these explicitly in the report dict)

Synthetic-anchored findings (``is_synthetic=True``) all share
uri="SECURITY.md" and have no real start_line. Hashing by
(rule_id, "SECURITY.md") alone would collapse every locationless
finding of the same CWE into a single alert. To distinguish them,
the synthetic path adds the class keyword extracted from the title
(same logic ``_class_fingerprint`` uses). The class keyword
catalogue is closed and stable, so cross-run identity holds — same
vulnerability class on the same rule_id always lands on the same
hash, and two different classes on the same rule_id don't collide.
Synthetic-anchored findings (``is_synthetic=True``) have no real
source location. Hashing by rule_id alone would collapse every
locationless finding of the same CWE into a single alert. To
distinguish them, the synthetic path adds the class keyword
extracted from the title (same logic ``_class_fingerprint`` uses).
The class keyword catalogue is closed and stable, so cross-run
identity holds — same vulnerability class on the same rule_id
always lands on the same hash, and two different classes on the
same rule_id don't collide.

Returns None when no anchor is available AND not synthetic.
"""
primary_physical = _first_physical_location(locations)
uri = ""
start_line: int | None = None
if primary_physical:
primary_physical = _first_physical_location(locations)
if is_synthetic:
uri = _LEGACY_SYNTHETIC_FINGERPRINT_URI
elif primary_physical:
uri = (primary_physical.get("artifactLocation") or {}).get("uri", "") or ""
region = primary_physical.get("region") or {}
sl = region.get("startLine")
Expand All @@ -864,7 +870,7 @@ def _primary_fingerprint(
endpoint = _string_value(report.get("endpoint")) or ""
route = f"{method.upper()} {endpoint}".strip() if (method or endpoint) else ""

if not uri and not route:
if not uri and not route and not is_synthetic:
return None

parts = [f"rule:{rule_id}"]
Expand Down
54 changes: 48 additions & 6 deletions tests/test_sarif.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import json
from typing import TYPE_CHECKING, Any

from strix.report.sarif import write_sarif
from strix.report.sarif import _primary_fingerprint, write_sarif


if TYPE_CHECKING:
Expand Down Expand Up @@ -61,13 +61,30 @@ def test_write_sarif_tool_version_is_reported(tmp_path: Path) -> None:


def test_write_sarif_locationless_finding_is_anchored_not_dropped(tmp_path: Path) -> None:
# A finding with no code location must still appear (anchored to a stable
# fallback), never be silently dropped from the report.
# A finding with no code location must still appear at a stable tracked
# repository anchor, never be silently dropped from the report.
write_sarif(tmp_path, [_finding(id="vuln-0002", code_locations=None)])
results = _read(tmp_path)["runs"][0]["results"]
assert len(results) == 1
uri = results[0]["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]
assert uri == "SECURITY.md"
assert uri == "README.md"


def test_write_sarif_synthetic_fingerprint_preserves_legacy_identity(tmp_path: Path) -> None:
finding = _finding(code_locations=None)
write_sarif(tmp_path, [finding])
emitted = _read(tmp_path)["runs"][0]["results"][0]["partialFingerprints"][
"primaryLocationLineHash"
]

legacy = _primary_fingerprint(
"CWE-89",
finding,
[{"physicalLocation": {"artifactLocation": {"uri": "SECURITY.md"}}}],
is_synthetic=True,
)

assert emitted == legacy


def test_write_sarif_fingerprint_stable_across_title_rewording(tmp_path: Path) -> None:
Expand Down Expand Up @@ -165,10 +182,17 @@ def test_write_sarif_omits_fixes_without_fix_pairs(tmp_path: Path) -> None:


def test_write_sarif_adds_logical_location_for_endpoint(tmp_path: Path) -> None:
# DAST findings hang off an endpoint; it must be preserved as a logical
# location so the finding keeps an addressable anchor.
# Findings with source locations retain them and add the endpoint as a
# logical location, so code scanning can navigate to both.
write_sarif(tmp_path, [_finding(endpoint="GET /api/users/{id}")])
locations = _read(tmp_path)["runs"][0]["results"][0]["locations"]
physical = [loc["physicalLocation"] for loc in locations if "physicalLocation" in loc]
assert physical == [
{
"artifactLocation": {"uri": "app.py"},
"region": {"startLine": 4},
}
]
logical = [
entry
for loc in locations
Expand All @@ -178,6 +202,24 @@ def test_write_sarif_adds_logical_location_for_endpoint(tmp_path: Path) -> None:
assert logical == [{"fullyQualifiedName": "GET /api/users/{id}", "kind": "endpoint"}]


def test_write_sarif_endpoint_finding_without_code_location_uses_synthetic_anchor(
tmp_path: Path,
) -> None:
# Endpoint-only DAST findings need a tracked physical anchor for
# code-scanning, while preserving the endpoint as logical context.
write_sarif(tmp_path, [_finding(code_locations=None, endpoint="POST /api/upload")])
locations = _read(tmp_path)["runs"][0]["results"][0]["locations"]
physical = [loc["physicalLocation"] for loc in locations if "physicalLocation" in loc]
assert physical == [{"artifactLocation": {"uri": "README.md"}}]
logical = [
entry
for loc in locations
for entry in loc.get("logicalLocations", [])
if entry.get("kind") == "endpoint"
]
assert logical == [{"fullyQualifiedName": "POST /api/upload", "kind": "endpoint"}]


def test_write_sarif_synthetic_finding_falls_back_to_resource_logical_location(
tmp_path: Path,
) -> None:
Expand Down